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

DM-7069: Port to Python 3 #19

Merged
merged 18 commits into from Aug 9, 2016
Merged

DM-7069: Port to Python 3 #19

merged 18 commits into from Aug 9, 2016

Conversation

timj
Copy link
Member

@timj timj commented Jul 29, 2016

This PR is for Python 3 work and other cleanups.

@timj timj force-pushed the tickets/DM-7069 branch 2 times, most recently from cf841f3 to 5eb4cbe Compare August 6, 2016 04:29
@timj timj force-pushed the tickets/DM-7069 branch 3 times, most recently from 3b6e71e to 7cf2063 Compare August 7, 2016 17:51
@@ -65,7 +65,7 @@ def getDefaultLevel(self):
return 'visit'

def getKeys(self, datasetType, level):
return {'filter': types.StringType, 'visit': types.IntType}
return {'filter': bytes, 'visit': int}
Copy link
Contributor

Choose a reason for hiding this comment

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

why bytes here and not str? (seems wrong, but maybe it's a new python3 thing I don't know about yet?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you. With so many changes it's easy to miss some. futurize knows that types.StringType is a synonym for bytes and since StringType does not exist on python3 it just buts bytes in there.

Copy link
Member Author

Choose a reason for hiding this comment

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

and of course this fixes my final test failure on Python 3.

@timj timj force-pushed the tickets/DM-7069 branch 2 times, most recently from 3cb2a7c to f059b03 Compare August 9, 2016 00:07
timj added 14 commits August 8, 2016 21:47
With some manual tweaks where futurize doesn't quite do the
right thing.

ReadProxy can not inherit from future object as this breaks the proxy
on Python 2.
The interfaces are compatible so this just changes the namespace.
On Python 3 str has __iter__ so all the tests that are checking
for an iterable in the codebase have to also be careful not to
include str by mistake. Refactor the code that looks for __iter__
to use routines from utils where possible. Where not possible
add instance checks for string.
Use __file__ to find the test file and then derive repository
locations from it.
Future object does not have an __init__ method so we have to catch
that case in the MRO.
Python 3 collections.UserDict is iterable but not on Python 2.
Python 2 requires we use UserDict.IterableUserDict so try that
first and then revert to collections.
This allows the tests to be visible to the test runner and is better
than globally disabling all testing in the file and hiding the tests.
Futurize converted types.StringType to bytes (technically correct
on python2) when it should have converted to str.
The next() method is just a method that iterates but is
not providing an iterable interface allowing the next() function
to work.
pytest assumes that classes with names starting with Test are test
classes that should be analyzed for test methods. When it can't
find any it warns that something might be wrong. Rename the tests
to remove the warnings.
@timj timj merged commit def6809 into master Aug 9, 2016
@ktlim ktlim deleted the tickets/DM-7069 branch August 25, 2018 06:14
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.

None yet

2 participants