-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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
CI: Unpin NumPy #23465
CI: Unpin NumPy #23465
Conversation
Hello @TomAugspurger! Thanks for submitting the PR.
|
Scipy import error on travis |
Codecov Report
@@ Coverage Diff @@
## master #23465 +/- ##
==========================================
+ Coverage 92.23% 92.25% +0.02%
==========================================
Files 161 161
Lines 51197 51176 -21
==========================================
- Hits 47220 47214 -6
+ Misses 3977 3962 -15
Continue to review full report at Codecov.
|
I don't really understand the scipy failure. I can't reproduce it locally right now. I pushed another commit to see if it fails again. |
This reverts commit e5eab3d.
OK, able to reproduce locally (on a Mac) with the latest wheels from https://7933911d6844c6c53a7d-47bd50c35cd79bd838daf386af554a83.ssl.cf2.rackcdn.com:
@rgommers do you know where the configuration for the scipy wheels uploaded to https://7933911d6844c6c53a7d-47bd50c35cd79bd838daf386af554a83.ssl.cf2.rackcdn.com lives? Or do you know offhand what versionsof NumPy and Cython those are compiled with? |
return not (safe_import('scipy.stats') and | ||
safe_import('scipy.sparse') and | ||
safe_import('scipy.interpolate') and | ||
safe_import('scipy.signal')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure why, but importing here should avoid the test failure. I suspect that whatever modifications pytest makes to the warningsfilter isn't active at this point (in the fixture), so the warning about the dtype size changing isn't elevated to an error.
That should be https://github.com/MacPython/scipy-wheels#how-it-works. Haven't checked, but it should be using the lowest supported NumPy version (same as for official releases, for ABI compatibility). Cython version will all be the same, not sure which version (check CI scripts). |
Thanks for the pointer. From https://travis-ci.org/MacPython/scipy-wheels/jobs/432291485, it seems like Cython 0.26.1 and NumPy 1.14.5 are used. And FWIW, I get the same warning / error with from |
@jreback if you have any thoughts, otherwise I'll merge tomorrow morning so we're testing against numpy master again. |
lgtm. |
@@ -410,7 +410,9 @@ def first_not_none(values): | |||
if (isinstance(v.index, MultiIndex) or | |||
key_index is None or | |||
isinstance(key_index, MultiIndex)): | |||
stacked_values = np.vstack(map(np.asarray, values)) | |||
stacked_values = np.vstack([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case you didn't follow the issue, the original failure came from us passing generator expressions to vstack
, and the new __array_function__
stuff in NumPy dev exhausted those generators. So we just build a list instead (which NumPy was doing anyway)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep saw that
thanks! |
Closes #23172