Apply python-future for Python 3.4 support #1221

alexsavio · 2015-09-17T11:49:49Z

No description provided.

alexsavio · 2015-09-17T13:34:40Z

For Python 3 I need to futurize prov as well, which is in https://github.com/trungdong/prov/archive/rdf.zip
Unless I can get nipype to work with the new version. This is not a big issue for now, I will keep trying Python 2.7 to pass the tests.

satra · 2015-09-17T14:06:23Z

@alexsavio - don't worry about prov for now. in fact if you want you can strip it out for now or use dummy functions in provenance.py

i'm working on updating nipype's prov support to be compatible with current master which already supports py3.

alexsavio · 2015-09-18T07:49:21Z

FAIL in both versions: nipype.algorithms.tests.test_splitmerge.test_split_and_merge

alexsavio · 2015-09-18T22:11:29Z

If I remove dumps and loads in getsource and create_function_from_source, it fixes many tests in pipeline.test.test_utils, this makes the function_str to be str instead of bytes in Python 3. In addition, assigning bytes to a traits.Str raises an error.
Is there any reason that I'm missing for you to be using "dumped" strings in function_str in Function?

satra · 2015-09-21T14:46:33Z

@alexsavio - the thought process behind using pickle is to ensure that the contents of function_str could be hashed properly and also turned into a python function that can be executed. if there is a better way to achieve the same effect, we can definitely try it out.

alexsavio · 2015-10-01T20:49:42Z

I have been busy with life. I'm currently stuck with unicodes, strings vs. Traits.
For example: nosetests -s nipype.interfaces.tests.test_base:test_TraitedSpec
In Python3 it works, now that everything is a unicode string, even what Traits returns, e.g., from get_hashval().
On Python2 the string literals are now unicode (thanks to from __future__ import unicode_literals) but TraitedSpec.get_hashval() is returning str. There are a few options to fix this, but I can't decide yet which one is better.

bcipolli · 2015-10-09T08:19:23Z

@alexsavio can you rebase on current master?

bcipolli · 2015-10-09T08:21:49Z

If I remove dumps and loads in getsource and create_function_from_source, it fixes many tests

This is a place where using six would make things easy; they take care of the import location change for cPickle...

alexsavio · 2015-10-09T08:27:21Z

Hi @bcipolli, do you mean merging the current master to this PR?

bcipolli · 2015-10-09T08:29:16Z

git rebase upstream/master. It replays your commits on top of the current master, rather than merging master on top of your changes.

alexsavio · 2015-10-09T08:35:58Z

Like this?

bcipolli · 2015-10-09T08:36:51Z

This branch has conflicts that must be resolved - have to fix this. Can you make me a collaborator on your repo?

alexsavio · 2015-10-09T08:41:35Z

done

alexsavio · 2015-10-09T08:42:07Z

I know, I haven't fixed everything yet

bcipolli · 2015-10-09T08:44:11Z

@alexsavio I will try to rebase. I added some questions from reviewing some commits. Will let you know how it goes!

bcipolli · 2015-10-09T10:02:59Z

I believe the Trait error is because environ = traits.DictStrStr, which likely requires a str (and not unicode). Will try to read a bit about how others have worked around things, if this is the issue.

alexsavio · 2015-10-09T10:03:53Z

Yes, definitely that is the case. Thanks @bcipolli!

bcipolli · 2015-10-09T10:05:44Z

Temporary workaround is to relax from DictStrStr to Dict. I think the most important thing is to get something merged (since there are so many potential merge conflicts), then open issues for things like this to be solved before the next release...

bcipolli · 2015-10-09T10:11:39Z

@alexsavio I force-pushed the rebase to your branch; you will need to rename/remove your current local branch to work on the force-pushed one.

I will do a bit more investigation on other errors, now that the TraitError workaround is in place. We can revisit that later; looks like there are plenty of errors to address in Py2 and Py3 in the meantime.

bcipolli · 2015-10-09T14:45:01Z

@alexsavio I think the strategy of applying changes globally (e.g. traits.Str => traits.Unicode,from future import unicode_literals`) is going to be problematic.

I think we need a Py23Str that is traits.Str for py2, traits.Unicode for py3
I think literals should not be unicode, as that means the calling code will have to be using unicode exclusively, and we can't guarantee that--users using py2 will be passing values to compare that are bytes.

I made the first change. The second one, however, I am starting to think should be removed everywhere. Otherwise, I don't think we have a hope for Py2 compatibility. What do you think?

alexsavio · 2015-10-09T14:58:51Z

@bcipolli Thanks!

Maybe you are right. I did the from future import unicode_literals because it was the recommendation in python-future.

Of course I realised it could be problematic once I stomped on the Traits failures. That is why I was asking. I suppose we have also the option to use six for strings (so, remove all the unicode_literals from the code and fix everywhere) or implement the Traits.Py6Str or Traits.Py23Str which was something in my mind, but I needed a hand to decide.

bcipolli · 2015-10-09T15:06:37Z

See the "Drawbacks" here, particularly the "user code" example:
http://python-future.org/imports.html#should-i-import-unicode-literals

My intuition says to try and migrate without unicode-literals. Is there a specific commit we can remove to try it out (rather than adding a commit to delete the code insertion)?

alexsavio · 2015-10-09T15:11:50Z

I guess you are right.
Sadly I used the command futurize, which makes many different changes not only the unicode_literals import.
This was done in the first commits that say 'Futurize ...'.
I can only see that the best thing is to add a commit to delete these imports. :/

bcipolli · 2015-10-09T17:05:49Z

@alexsavio I made a commit to remove unicode_laterals. Now, checks such as this fail: isinstance\(.*, str\). This should migrate to six (isinstance\(.*, string_types\)). Do you have time for that? I have to move on to other work today.

bcipolli · 2015-10-09T20:24:31Z

@alexsavio I need a break from my work, so I will try out the above replacement.

alexsavio · 2015-10-09T20:32:06Z

Hi @bcipolli I was out. How is it going? can I help?

Enh/py3k fix

satra · 2015-10-11T20:10:49Z

@bcipolli - the latest commits should take care of all the issues - let's see what circle says.

also if you have a chance to scroll through this commit that would be great.
alexsavio@df8fbaf

chrisgorgo · 2015-10-11T20:39:02Z

circle.yml

@@ -9,6 +9,7 @@ dependencies:
    - "~/examples/fsl_course_data"
  override:
    - pip install --upgrade pip
+    - pip install -r requirements.txt


Why do we need this? All dependencies should be in the setup.py file and picked up by

pip install -e .

I recommend removing requirements.txt from circle.yml. Keeping it may lead to tests passing, but pip-installed packages failing to run because some dependency was in in requirements.txt, but was left out of setup.py.

satra · 2015-10-12T02:57:21Z

@chrisfilo - it's hard to read the circleci output, but am i understanding that the only error is in doc generation?

chrisgorgo · 2015-10-12T03:02:00Z

both nosetest and doc generation are currently failing on this branch

nosetest:

======================================================================
FAIL: nipype.interfaces.spm.tests.test_base.test_spm_path(<type 'unicode'>, <type 'str'>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ubuntu/virtualenvs/venv-system/local/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/ubuntu/virtualenvs/venv-system/local/lib/python2.7/site-packages/numpy/testing/utils.py", line 354, in assert_equal
    raise AssertionError(msg)
AssertionError: 
Items are not equal:
 ACTUAL: <type 'unicode'>
 DESIRED: <type 'str'>

documentation:

/home/ubuntu/nipype/doc/users/examples/fmri_ants_openfmri.rst:35: ERROR: Unexpected indentation.
/home/ubuntu/nipype/doc/users/examples/fmri_ants_openfmri.rst:49: ERROR: Unexpected indentation.

and many similar

bcipolli · 2015-10-12T06:09:57Z

Thanks @chrisfilo . I was able to reproduce the doc build issues locally; they are due to a missing blank line between the file docstring and first import. I can fix those.

The other issue is due to output streams coming out unicode on python 2/3. Haven't figured out why this is yet.

bcipolli · 2015-10-12T13:58:21Z

@satra @alexsavio tests are passing!! :) One last issue I thought of last night; the use of loads/dumps. I believe some were removed (per the discussion above) without replacement; please take a look for example at:
alexsavio@6ceb87e
alexsavio@f531ed6

to confirm this won't cause any problem.

satra · 2015-10-12T14:02:54Z

@bcipolli - i'm going to open an issue for loads/dumps - i have no recollection why i wrote it with pickling. so we will monitor it in case we run into trouble, but otherwise i think this is ready for merging.

@chrisfilo - could i leave it to you to take any last look and hit the merge button?

chrisgorgo · 2015-10-13T04:22:18Z

Did you guys try to switch Circle to python 3 for one commit and see if the workflows still work? That would convince me that we are indeed P3 compatible.

bcipolli · 2015-10-13T04:25:17Z

@chrisfilo no, we didn't. Any suggestion how to do that?

chrisgorgo · 2015-10-13T04:28:46Z

According to this https://circleci.com/docs/language-python you need to add the following lines to circle.yml:

machine:
  python:
    version: 3.5.0

There are more versions available: https://circleci.com/docs/environment#python

bcipolli · 2015-10-13T13:29:14Z

@chrisfilo could you help look over the failures? There were four. I looked over the text, and it wasn't clear to me what the problem might be, nor how to reproduce them locally.

The easiest one to reproduce locally seemed to be: nipype.interfaces.tests.test_io:test_s3datagrabber_communication, which I guess just required installing boto, but when I run the following command, it hangs for 10 minutes (before I kill it):

python3 which nosetests nipype.interfaces.tests.test_io:test_s3datagrabber_communication

chrisgorgo · 2015-10-13T19:42:02Z

The errors are indeed strange. I'll try rebuilding without cache. Hold tight it will take a while.

…or sphinx)

* check for NIPYPE_NO_MATLAB before doing code that may throw. * Give better error messages when tools (dot) not installed. * update gitignore for all built files.

bcipolli · 2015-10-15T15:45:42Z

Please see #1253

Apply python-future for Python 3.4 support

satra · 2015-10-15T19:21:53Z

woohoo!

alexsavio · 2015-10-15T19:23:19Z

:)

coveralls · 2017-06-29T21:50:18Z

Coverage decreased (-47.02%) to 25.084% when pulling 6406ace on alexsavio:enh/py3k into 11111cd on nipy:master.

alexsavio mentioned this pull request Oct 9, 2015

It does not work on Python 3.4 #1220

Closed

bcipolli force-pushed the enh/py3k branch from 8054c72 to 5d9bd45 Compare October 9, 2015 10:10

bcipolli force-pushed the enh/py3k branch from fe3fd9c to 08bf19f Compare October 9, 2015 17:17

bcipolli force-pushed the enh/py3k branch from 531a8e0 to 82abeb5 Compare October 9, 2015 20:36

satra and others added 3 commits October 11, 2015 16:01

fix: updated auto test generator to use relative imports

0bac3c7

fix: updated auto tests with relative imports

9b52c9d

Merge pull request #4 from satra/enh/py3k-fix

3a01d1a

Enh/py3k fix

chrisgorgo reviewed Oct 11, 2015
View reviewed changes

satra mentioned this pull request Oct 12, 2015

python 3 support removes loads dumps from function #1247

Closed

satra and others added 5 commits October 15, 2015 08:34

fix: changing setup.py and circleci.yml for requirements

20aedc6

fix: add blank line between file docstring and first import (needed f…

29d774b

…or sphinx)

fix: tweaks for building docs:

63566a9

* check for NIPYPE_NO_MATLAB before doing code that may throw. * Give better error messages when tools (dot) not installed. * update gitignore for all built files.

fix: make sure stdout/stderr are of python version's string type.

07d598a

Remove future from setup / setup_egg

6406ace

bcipolli force-pushed the enh/py3k branch from 2d7d756 to 6406ace Compare October 15, 2015 15:34

bcipolli mentioned this pull request Oct 15, 2015

Python 3.4 support - fix tests in circleci #1253

Closed

chrisgorgo added a commit that referenced this pull request Oct 15, 2015

Merge pull request #1221 from alexsavio/enh/py3k

7e3393e

Apply python-future for Python 3.4 support

chrisgorgo merged commit 7e3393e into nipy:master Oct 15, 2015

alexsavio deleted the enh/py3k branch November 16, 2015 14:13

Apply python-future for Python 3.4 support #1221

Apply python-future for Python 3.4 support #1221

Uh oh!

Conversation

alexsavio commented Sep 17, 2015

Uh oh!

alexsavio commented Sep 17, 2015

Uh oh!

satra commented Sep 17, 2015

Uh oh!

alexsavio commented Sep 18, 2015

Uh oh!

alexsavio commented Sep 18, 2015

Uh oh!

satra commented Sep 21, 2015

Uh oh!

alexsavio commented Oct 1, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

alexsavio commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

alexsavio commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

alexsavio commented Oct 9, 2015

Uh oh!

alexsavio commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

alexsavio commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

alexsavio commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

alexsavio commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

bcipolli commented Oct 9, 2015

Uh oh!

alexsavio commented Oct 9, 2015

Uh oh!

satra commented Oct 11, 2015

Uh oh!

chrisgorgo Oct 11, 2015

Choose a reason for hiding this comment

Uh oh!

satra commented Oct 12, 2015

Uh oh!

chrisgorgo commented Oct 12, 2015

Uh oh!

bcipolli commented Oct 12, 2015

Uh oh!

bcipolli commented Oct 12, 2015

Uh oh!

satra commented Oct 12, 2015

Uh oh!

chrisgorgo commented Oct 13, 2015

Uh oh!

bcipolli commented Oct 13, 2015

Uh oh!

chrisgorgo commented Oct 13, 2015

Uh oh!

bcipolli commented Oct 13, 2015