-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
Revert "CI: workaround numpydev bug (#29433)" #29553
Conversation
This reverts commit b4adb71.
Hmm a few warnings snuck in. @jorisvandenbossche is fixing some in #29549 . I won't have time today or tomorrow for the rest. Anyone able to push fixes to my branch? |
at least some of these look like the result of #29517; ill see what i can do about it |
c0c4c77
to
5c38be7
Compare
pushed a hopeful-fix, will see how it goes |
lgtm. any perf implications? (IOW now we have to catch moar warnings)? |
It shouldn't be any worse than the previous solution we had in place that did specific dtype checks to prevent the warnings. |
I'm not sure we should be filtering the warnings like this. IIUC this may break in the future when NumPy enforces their deprecation. I don't know enough of the context for https://github.com/pandas-dev/pandas/pull/29517/files, but we don't want the behavior to silently change for the user. If we like the change from NumPy, then we should not filter the warnings. If we don't like it (say because we need more flexible dtype comparison) then I think we need to restore some of the checks in #29517 (again, I haven't read through the changes there in detail). |
Tom makes a fair point. I'd be OK with reverting #29517 since having this build is clearly more important. |
what if we filter on the specific warning, is that better? |
just pushed a commit that restored the checks that were removed by #29517. I may try to remove them again someday, but getting this build back up is a higher priority |
do we need this? |
"This" being the commit Brock pushed up in #29553 (comment)? I think so... That at least gets us back to where we were, and gets us testing against numpydev again. |
right. i guess if you can rebase this and see |
Merged master. |
this should be good to go? |
thanks @TomAugspurger and @jbrockmendel maybe open a followup issue to re-remove this at some point if possible? |
This reverts commit b4adb71.
Closes #29432