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

making it python 3 compatible_REVISED #194

Closed
wants to merge 10 commits into from

Conversation

JoostJM
Copy link
Collaborator

@JoostJM JoostJM commented Feb 14, 2017

without breaking the python 2 compatibility (#178), adding the future imports to keep others clean and behavior particularly division consistent.

The current changes should change or break nothing with python 2 execution and only make the code itself runnable on 3.

supersedes #188

Additional changes to previous PR:

cc @Radiomics/developers, @kmader

kmader and others added 7 commits February 14, 2017 10:15
…mpatibility, adding the future imports to keep others clean and behavior particularly division consistent
…, adding a new notebook for simple tests, fixing typo in safe_xrange, most tests pass now, but still differences, probably because SimpleITK, numpy, and PyWavelets have changed.
In python 3, the zip function returns a 'zip' object, similar to python generators. This does not change it's implementation in python for loops, but it is not evaluated if it is used to generate a numpy array. Therefore, add a cast to list, which evaluates the object, before assigning it to the numpy array.

Also, use relative imports in tests.
Finally, fix some print-function bugs in the 'bin' and 'batchexamples' folder and rerun the python notebooks, so the output reflects the changes made.
Some functions expect string types (such as in nose-testing), but python 3.5 by default provides unicode strings. Add forced casts where necessary.
Also, 'wb' mode requires strings to be encoded as bytes, not string in python 3.5. The fix did not work in this case. Change mode to 'w', which accepts regular strings.
Additionally, safe_cmp produced errors for dictionary comparison in python 3.5, replace by `==` comparison.

Finally, update layout in testing files (such as replace `!=` by `is not` in comparisons to `None`).
Update feature name in testing output to also reflect the feature class.
In python 3.5 GLRLM results differed from baseline. This due to the different implementation of the `filter()` function, which returns an iterator instead of a list. In GLRLM calculation, there where 2 iterations over this iter-object (one for flat-angle check, one for matrix calculation), causing it to break in python 3.5. Fix by merging the 2 iterations into one loop.

Additionally, simplify the filter function.
In python 3, `xrange` and `basestring` are undefined names (python 2 specific). Therefore, these will produce an F821 flake8 error on testing in python 3 environment. Add this error to the ignore list to let it pass the flake 8 test.

Alternative: add `radiomics/__init__.py` to the excluded files (`xrange` and `basestring` only used here).
@jcfr
Copy link
Collaborator

jcfr commented Feb 14, 2017

@JoostJM I ended up working on this earlier tonight. I will push the topic shortly.

@jcfr
Copy link
Collaborator

jcfr commented Feb 14, 2017

Let's also note that my topic is based on the current master

@JoostJM
Copy link
Collaborator Author

JoostJM commented Feb 14, 2017

@jcfr Great! Also, currently the python 3 tests are failing for python 3.3, python 3.4 and higher work. The error is that the test try to install the latest version of numpy (as the requirement is >=1.9.2). However, numpy 1.12.0 requires python 2.7 or python >= 3.4.

@JoostJM
Copy link
Collaborator Author

JoostJM commented Feb 14, 2017

superseded by #196

@JoostJM JoostJM closed this Feb 14, 2017
@jcfr jcfr deleted the add-python3-compatibility branch February 15, 2017 23:05
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.

3 participants