Various fixes for Python 2 compatibility - #33
Conversation
Signed-off-by: Stephen L. <lrq3000@gmail.com>
|
I am trying to pull this PR to test locally... I am having the python2.7 compatibility issues... I tried some variants, but wasn't sure of the syntax to try and test this.. |
|
Yes this is normal, it's because widgetti/ipyvolume#103 needs also to be pulled in ipyvolume, else it won't work. However, even if with this PR and the one for ipyvolume the niwidgets are indeed displayed with no Python compatibility error, I could not display correctly my maps, I just get a pinkish colored volume (but maybe that was because of my mask). But this would be the purpose of another PR I guess, so I leave this issue out for now. |
|
Hi there, thanks for submitting this PR. I'm actually planning to remove the reliance on pathlib entirely, since much of In the long run, I'm not sure how much I will invest into supporting python 2. Is there a particular reason you're still using it, such as a neuroimaging package that doesn't support python 3? |
|
@janfreyberg Yes I keep on using Python2 merely because some packages still do not play well with Python3 but I think most packages are migrating now so it should be fine if you move to support Python3 only :-) |
|
Cool, thanks for the feedback. I'll try to not put any obviously breaking python 3 only features in anyways. |
This PR fixes Python 2 compatibility.
It is dependent on this Py2 compatibility fix in ipyvolume: widgetti/ipyvolume#103
Also I'm not sure how to fix the following line elegantly:
https://github.com/nipy/niwidgets/compare/master...lrq3000:py2fix?expand=1#diff-c4e3c3c90dfe314c2eaad3135f7992d4L46
But in any case, in Python 2, os.stat() and thus os.file.exists() expect a str, not a Path object. Maybe we can do a branching depending on Python 2 or Python 3?