-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
Fix numpy vstack on generator expressions #12467
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
Conversation
oops, wrong branch. Let me rebase. |
shouldn't numpy avoid breaking changes? ;) |
e9b8301
to
b44107a
Compare
I agree but this is an easy workaround in the mean time. |
Note sure about the 0.20.1 milestone but probably safe to backport. |
ok but this change needs to be made in many more places, right? |
Hum you are rights, I have just fixed a specific module but there are many others in the travis report. I reported the issue upstream. Let's see what they think of it. I have to run now. Feel free to push the missing fix into that PR directly if you want. |
Related to numpy/numpy#12263 and the NEP implementing array function protocol. For SciPy we did a similar fix by switching to list comprehensions. @shoyer noted in the above issue the possibility of re-enabling the behavior for now, but eventually raising a |
I guess this is up for grabs if anyone wants to finish it as we should try to avoid the |
b44107a
to
e52ca6d
Compare
I think I fixed them all (when running the tests locally with numpy master). |
Miscellaneous | ||
............. | ||
|
||
- |Fix| Make sure to avoid raising ``FutureWarning`` when calling |
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.
Seems that FutureWarning is only a proposal (not implemented) in numpy?
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.
It's pretty likely to be implemented. If not we can always update our changelog later.
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.
Could you give me some pointers about where the proposal/discussion about FutureWarning
and VisibleDeprecationWarning
in numpy
is happening?
It'll effect the semantics behind sklearn.utils.testing.assert_no_warnings
: d6972a9
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.
see numpy/numpy#12263 mentioned above.
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.
The FutureWarning PR has been merged into numpy master as well: https://github.com/numpy/numpy/pull/12280/files
looks good. haven't checked for completeness but cron job will do that for us. |
FYI, there's only a joblib issue in Travis cron job now :) |
* Workaround vstack issue with genxp * Use list comprehensions instead of genexps with np.vstack * Add changelog entry.
* Workaround vstack issue with genxp * Use list comprehensions instead of genexps with np.vstack * Add changelog entry.
* Workaround vstack issue with genxp * Use list comprehensions instead of genexps with np.vstack * Add changelog entry.
* Workaround vstack issue with genxp * Use list comprehensions instead of genexps with np.vstack * Add changelog entry.
This reverts commit 71607a8.
This reverts commit 71607a8.
* Workaround vstack issue with genxp * Use list comprehensions instead of genexps with np.vstack * Add changelog entry.
This workaround should fix the test failing with numpy master (CRON job on travis):
https://travis-ci.org/scikit-learn/scikit-learn/builds/446632038