-
Notifications
You must be signed in to change notification settings - Fork 532
python 2.7 string issue in plugins #1621
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
Comments
Ok, I'll look at this now |
Thanks but in fact in seems something more global, the future newstr seems to interact with all traits string type checking. Each string that comes from os.environ causes crash.
I do not know if there is a simple way to change this except removing all future |
That's the thing: without |
ok, I am just surprised that it got merged in master considering that it breaks a lot of nipype interfaces/plugins when used in 2.x environment. Or am I the only one to experience problems with 2.x? |
It doesn't break a lot nipype interfaces. We are constantly checking the circleCI examples for both 2.7 and 3.5 (https://circleci.com/gh/nipy/nipype/1475). The alternative would be stick with 2.7 forever. |
I have been running nosetest on nipype in my freshly cleaned conda 2.7 environment, and in fact it seems that any inputs that requires string from os.environ (most inheriting from CommandLine) crashes, which is a huge problem for most pipelines that I use.
This will solve my case for environ problem, nosetests crashes for unicode string in other places, but I am less concerned about that for now. |
This is weird... Are you testing current master? Travis was suppose to run tests in python 2.7, 3.4 and 3.5 to ensure all of those are supported. See: https://travis-ci.org/nipy/nipype |
yes I am testing the current master |
Since I updated, I have tested on my regular python and also anaconda. |
I wonder if this could have something to do with the version of futures/six on your system. |
six==1.10.0 |
It's the same as on Travis :/ https://s3.amazonaws.com/archive.travis-ci.org/jobs/159706130/log.txt |
With nipype installed from conda and updated with current master:
|
@bpinsard may you hand over me a simple workflow that fails for you, using the SGE plugin? I have already set up a Docker image with SGE and I can try to replicate the problem. I will think of some tests for the SGE plugin as well. Current coverage is not very comprehensive:
|
just to show the output of the command line above
looking at the output of The plugins do not yield errors in nosetest because the environ is not an input, but is set when job is submitted, so I do not know if this is possible to write test that would work without SGE installed. |
I think there is something really weird happening.
Then I go into debug and run the exact same line that crashed, it crashes with (type str):
When I import os an check type of os.environ values
So when it is in the nipype code running, all my strings (even the one written in nipype code, such as 'SUBJECTS_DIR', as well as os.environ) get converted to unicode, which is the problem when type checking is done in traits. |
Does it replicate if you do
? Thanks |
yes it replicate the bug |
ok just to update on this issue, that is in fact not solved for me. So as to try the exact line that fail for string being unicode I did a small script:
Which outputs:
When I remove the
So it seems that future converts strings defined in code in unicode. How should I force this string to be newstr or newbytes? What is the rationale of importing unicode_literals everywhere? Thanks |
the above PR fixes the bug... |
The rationale behind importing Regarding your examples, how is that possible that your in your first example you have two different types? I get unicode for both strings. |
does os.environ includes future? I don't know. |
I mean:
Maybe you meant:
? |
oups sorry, I pasted then updated my code, you got it correct it was
|
with recent updates, I get the following error using SGE plugin:
It seems that the traits definition uses the future newstr which does not match with what os.environ contains (old str). It is certainly the same for other plugins, and this is the only places where environ is fed to CommandLine in whole nipype.
Any idea how to fix this?
Thanks
The text was updated successfully, but these errors were encountered: