Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Python3 compatibility #18

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yungchin
Copy link

This set of changes to the Python implementation is sufficient for tests to pass both on 3.4 and 2.7 - if support for older versions is desirable, a bit more work would need doing.

I've aimed to keep changes as minimal as possible, and functional behaviour unchanged.

Tests pass against Python 2.7 and 3.4 so far.  All changes were applied
manually (that is, without any conversion tools), so I believe the
meaning and intent of all functions have been preserved.

Signed-off-by: Yung-Chin Oei <[email protected]>
r += chr(random.SystemRandom().getrandbits(8))
return hexify(r)
r = random.SystemRandom().getrandbits(256)
return hex(r)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, wait a second. I was trying to avoid messing around with types with this change, but this change doesn't ensure the returned str is of length 32.

yungchin added 2 commits July 19, 2016 13:49
The parent commit tried a silly substition for `hexify`, seeking to work
around the problem that `chr` has a different return type in py3.  This
brings back `hexify` to ensure that the output of `redactable_rand` is
always as expected: a hexadecimal `str` of length 64.

Signed-off-by: Yung-Chin Oei <[email protected]>
This fix is only necessary to run the tests on a platform that does not
default to utf-8.  To remain py2.7 compatible, we needed to replace
`open()` as the built-in does not let you specify an encoding.

Signed-off-by: Yung-Chin Oei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant