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

Remove Python 2 dependency. #1775

Merged
merged 20 commits into from May 7, 2019

Conversation

@arokem
Copy link
Member

commented Mar 15, 2019

Implements as discussed in #1731

Also, travis-ci/travis-ci#9815 seems to now be resolved.

@arokem

This comment has been minimized.

Copy link
Member Author

commented Mar 15, 2019

These errors: https://travis-ci.org/nipy/dipy/jobs/506703626#L2364

Are addressed in nibabel (nipy/nibabel#719)

I am not sure why they appear only on the machine with coverage turned on, though.

@arokem

This comment has been minimized.

Copy link
Member Author

commented Mar 15, 2019

We should wait for a nibabel release to rebase this :-)

@effigies

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

Coming up on Monday. :-)

@arokem arokem force-pushed the arokem:drop_py2 branch from 41558b8 to fd7d843 Apr 1, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Apr 1, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@46e8747). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1775   +/-   ##
=========================================
  Coverage          ?   84.07%           
=========================================
  Files             ?      118           
  Lines             ?    14489           
  Branches          ?     2283           
=========================================
  Hits              ?    12181           
  Misses            ?     1792           
  Partials          ?      516
Impacted Files Coverage Δ
dipy/info.py 100% <100%> (ø)
@arokem

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

Now, that it's Monday and Nibabel is released (👏 🎉 🍾), rebased to see what happens.

@effigies

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

Well, it was a Monday. Thanks for your patience.

@arokem

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

@skoudoro

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

should we use this PR to reconsider our minimal version as discussed here?

@arokem

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2019

@skoudoro
Copy link
Member

left a comment

Concerning the first build failing, I wonder whether Numpy 1.8.2 is compatible with python 3.7 because it fails to build it.
Concerning the 2 others build failing, see below, I think it should fix it.

Thank you again for this!

setup_egg.py Outdated Show resolved Hide resolved

@arokem arokem force-pushed the arokem:drop_py2 branch from 035babc to c9d89d0 May 3, 2019

@pep8speaks

This comment has been minimized.

Copy link

commented May 3, 2019

Hello @arokem, Thank you for updating !

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated at 2019-05-07 15:46:38 UTC
@arokem

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

There is a numpy wheel for 3.7 on pypi: https://pypi.org/project/numpy/#files

@skoudoro

This comment has been minimized.

Copy link
Member

commented May 3, 2019

There is a numpy wheel for 3.7 on pypi: https://pypi.org/project/numpy/#files

Not for our minimal version Numpy 1.8.2 as you can see here

@arokem

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

Oh yeah - gotcha.

Should we bump up our minimal numpy, so we can guarantee support for 3.7? Or provide support for 3.7 for only some versions of numpy?

I would vote for the latter

@skoudoro

This comment has been minimized.

Copy link
Member

commented May 3, 2019

I would vote for the latter

I will vote for that too if you explain to me how do you manage it? 😄

@arokem

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

This is what I was thinking for the CI.

For the requirements, I think that we would have to do something like what Adam did here in Cloudknot.

@skoudoro

This comment has been minimized.

Copy link
Member

commented May 3, 2019

Ok, I see, Nice! Thank you for that!

@arokem arokem force-pushed the arokem:drop_py2 branch from c3f44ad to 184bc34 May 3, 2019

@skoudoro

This comment has been minimized.

Copy link
Member

commented May 3, 2019

During your force-pushed it seems you have deleted this and this. Can you put them back?

Thanks!

@arokem

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

Whoopsie. I think that your change to setup_egg is superseded by ddfa2f4, but I cherry-picked back your dipy/info.py, because I am a git wizard.

@arokem arokem force-pushed the arokem:drop_py2 branch from 276539c to aa97c54 May 7, 2019

@arokem

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

Looks like Travis is all 💚 now! Let's see what Appveyor says.

@skoudoro

This comment has been minimized.

Copy link
Member

commented May 7, 2019

😍, look forward to press "merge"

@skoudoro skoudoro merged commit 2ce4059 into nipy:master May 7, 2019

4 checks passed

codecov/patch No report found to compare against
Details
codecov/project No report found to compare against
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@skoudoro

This comment has been minimized.

Copy link
Member

commented May 7, 2019

moving to python 3 only! Thanks @arokem! merging

@arokem

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

🎉 🍾 🌟

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.