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

MRG: use newer wheelhouse for installs #1186

Merged
merged 9 commits into from Mar 23, 2017

Conversation

Projects
None yet
4 participants
@matthew-brett
Member

matthew-brett commented Mar 6, 2017

We were using a custom wheelhouse, but now manylinux wheels are pretty
well supported; switch to using the standard pypi wheels plus some extra
wheels for older distributions.

This should address the need for cvxpy wheels - see #1180.

@arokem

This comment has been minimized.

Member

arokem commented Mar 7, 2017

Time to also drop Python 3.3?

@arokem

This comment has been minimized.

Member

arokem commented Mar 7, 2017

Looks like numpy is no longer supporting it? https://travis-ci.org/nipy/dipy/jobs/208408058

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Mar 7, 2017

I think this PR is correct now, but it has exposed a couple of test errors with latest numpy / matplotlib:

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Mar 7, 2017

I also dropped Python 3.3.

@arokem

This comment has been minimized.

Member

arokem commented Mar 7, 2017

See #1189, which addresses the negative integer powers. When that's merged you can rebase here.

@arokem

This comment has been minimized.

Member

arokem commented Mar 7, 2017

Also made a PR directly into this branch (matthew-brett#5), so that we can test this on Travis with the right version of numpy, before moving ahead with that.

@arokem arokem referenced this pull request Mar 7, 2017

Closed

Np1.12 #1189

@matthew-brett matthew-brett force-pushed the matthew-brett:new-wheelhouse branch 2 times, most recently from 29da2a4 to 0fd4bc6 Mar 7, 2017

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Mar 7, 2017

A couple more issues arising:

  • Python 3.6 failure : #1190;
  • Failure for current numpy master : #1191.

Otherwise, we're just waiting on #1188 .

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Mar 7, 2017

From my point of view, this is ready to merge when #1188 and #1190 are fixed.

@matthew-brett matthew-brett changed the title from WIP: use newer wheelhouse for installs to MRG: use newer wheelhouse for installs Mar 9, 2017

@arokem

This comment has been minimized.

Member

arokem commented Mar 14, 2017

This should be GTG after a rebase!

@matthew-brett matthew-brett force-pushed the matthew-brett:new-wheelhouse branch from 5a35d7e to 4f7cea7 Mar 14, 2017

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Mar 14, 2017

Done rebase - but is #1190 fixed yet?

@arokem

This comment has been minimized.

Member

arokem commented Mar 14, 2017

Oh - you're right 😞

Looks nasty.

@coveralls

This comment has been minimized.

coveralls commented Mar 14, 2017

Coverage Status

Coverage decreased (-0.004%) to 88.426% when pulling 4f7cea7 on matthew-brett:new-wheelhouse into 26bfae6 on nipy:master.

@arokem

This comment has been minimized.

Member

arokem commented Mar 22, 2017

Are we ready for this one yet?

matthew-brett and others added some commits Mar 6, 2017

MAINT: use newer wheelhouse for installs
We were using a custom wheelhouse, but now manylinux wheels are pretty
well supported; switch to using the standard pypi wheels plus some extra
wheels for older distributions.

We have to drop Python 3.3; latest numpy does not support it.
Index with integers.
Also don't use invalid values for arccos (0).
MAINT: add pre-release entry to build matrix
Test against pre-release wheels.
BF: another couple of Python 3 / numpy 1.12 fixes
Making integers for empty size argument, slicing.
MAINT: add Python 3.6 entry; --pre can fail
Add entry for Python 3.6.

Allow pre-release tests to fail.

matthew-brett added some commits Mar 7, 2017

BF: fix test for not None elements in array
Numpy > 1.12 will raise an error for equality of an array to None; use
numpy vectorize to do the same comparison.

@matthew-brett matthew-brett force-pushed the matthew-brett:new-wheelhouse branch from 4f7cea7 to 599e16c Mar 22, 2017

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Mar 22, 2017

Rebased.

@codecov-io

This comment has been minimized.

codecov-io commented Mar 22, 2017

Codecov Report

Merging #1186 into master will decrease coverage by <.01%.
The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master    #1186      +/-   ##
==========================================
- Coverage   85.93%   85.93%   -0.01%     
==========================================
  Files         219      219              
  Lines       26414    26414              
  Branches     2714     2714              
==========================================
- Hits        22700    22699       -1     
- Misses       3052     3053       +1     
  Partials      662      662
Impacted Files Coverage Δ
dipy/reconst/tests/test_csdeconv.py 99.38% <100%> (ø) ⬆️
dipy/workflows/tests/test_masking.py 91.66% <100%> (ø) ⬆️
dipy/info.py 100% <100%> (ø) ⬆️
dipy/core/tests/test_geometry.py 98.93% <100%> (ø) ⬆️
dipy/reconst/shm.py 86.84% <100%> (ø) ⬆️
dipy/reconst/mapmri.py 90.47% <66.66%> (ø) ⬆️
dipy/viz/tests/test_fvtk.py 91.35% <0%> (-1.24%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ea8af7...bee1df7. Read the comment docs.

@coveralls

This comment has been minimized.

coveralls commented Mar 23, 2017

Coverage Status

Coverage decreased (-0.004%) to 88.432% when pulling 599e16c on matthew-brett:new-wheelhouse into 0ea8af7 on nipy:master.

@arokem

This comment has been minimized.

Member

arokem commented Mar 23, 2017

So how should we handle the failures in the "pre" machine?

https://travis-ci.org/nipy/dipy/jobs/214060347

Should we go ahead and merge this and fix those on subsequent PRs? Given how long we go between releases, we should definitely fix these before making a release. But do we need to fix these before merging this?

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Mar 23, 2017

If it was up to me, I'd make sure the pre-release numpy checks were working, before the release, to give us some confidence that dipy won't be broken by the next release of numpy.

EDIT - sorry - this all fully agrees with what you just said.

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Mar 23, 2017

Having said that, I think this one's OK to merge, but with a plan to fix #1191 before the next release.

@coveralls

This comment has been minimized.

coveralls commented Mar 23, 2017

Coverage Status

Coverage decreased (-0.004%) to 88.432% when pulling b0332e0 on matthew-brett:new-wheelhouse into 0ea8af7 on nipy:master.

RF: set minimum nibabel dependency to 2.1.0
Note in requirements, info.py and .travis.yml.

@matthew-brett matthew-brett force-pushed the matthew-brett:new-wheelhouse branch from b0332e0 to bee1df7 Mar 23, 2017

@coveralls

This comment has been minimized.

coveralls commented Mar 23, 2017

Coverage Status

Coverage decreased (-0.004%) to 88.432% when pulling bee1df7 on matthew-brett:new-wheelhouse into 0ea8af7 on nipy:master.

@arokem

This comment has been minimized.

Member

arokem commented Mar 23, 2017

Yup. Makes sense. Merging.

@arokem arokem merged commit dd4a17b into nipy:master Mar 23, 2017

3 of 4 checks passed

coverage/coveralls Coverage decreased (-0.004%) to 88.432%
Details
codecov/patch 86.66% of diff hit (target 85.93%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +0.72% compared to 0ea8af7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@arokem arokem referenced this pull request Mar 23, 2017

Closed

TST: Test on Python 3.6 #1173

@arokem

This comment has been minimized.

Member

arokem commented Mar 23, 2017

Interesting. Looks like this also sped up Travis runs quite substantially. My impression is that it went down from ~3 hrs down to ~1 hr. It might also be something else.

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Merge pull request nipy#1186 from matthew-brett/new-wheelhouse
MRG: use newer wheelhouse for installs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment