-
-
Notifications
You must be signed in to change notification settings - Fork 26k
MNT More replacements of numpy aliases with built-in types #17707
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
MNT More replacements of numpy aliases with built-in types #17707
Conversation
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.
Thanks @alfaro96 !
Do we try to correct the one mentioned by Travis in this PR or it is preferable to merge as it is? |
I think it will be better to correct the errors shown by Travis before merging this PR to ensure that everything is properly working. |
…kit-learn into replace_deprecated_np_items
I have applied the corresponding changes. Let us wait for the green in |
Travis is failing because of these deprecations coming from I think we should ignore these warnings until |
True but it seems that we have some PicklingError :S |
This is what I am worried about 😕. |
…kit-learn into replace_deprecated_np_items
I think that this PR is ready to merge (we should handle the WDYT @glemaitre? |
Could you maybe explain where the pickling error comes from? |
The In particular, the exception raises in lines: scikit-learn/sklearn/cluster/_agglomerative.py Lines 883 to 886 in 41488fc
while The exact error is:
|
Does the
error make sense to you? Besides, It doesn't seem to be relevant to this PR since you're changing the |
IMHO, this error does not make sense (why I have applied a few of manual changes apart from Nevertheless, I would like to investigate this issue and a few more related with the |
right now the best thing to do is to isolate the issue and figure out what exactly the issue is. And if it's not related to your changes (which I suspect is the case) then we can safely merge this PR. The issue right now is that we're in the dark. Alternatively, if you confirm that the error exists on |
You are absolutely right, the issue seems quite strange and we need to check if the error comes from this PR or also exists on I will open a PR with the version of the |
Ok this is becoming tricky (see #17719). I'm happy to merge this one and have the pickling issue investigated after. |
WDYT @glemaitre |
Bad news. This error is not related with this PR, but needs further investigation. I will try to have a look and determine why this is happening. |
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.
Good then, let's merge this one. It'd be really nice if you could look into the pickling error.
pickling make me think that something is happening with cloudpickle maybe. ping @ogrisel :) |
From what I can see, the error seems to be related to This comes from the fact that import pickle
import numpy as np
print('numpy version:', np.__version__)
dt = np.dtype('float64')
print('class: ', dt.__class__)
try:
pickle.dumps(dt.__class__)
print('Can pickle dt.__class__')
except Exception as exc:
print('Exception while pickling dt.__class__')
print(exc) Output numpy-dev:
Ouptut numpy 1.19.0:
|
Thanks @lesteve for investigating this! I will submit a PR and trigger the nightly jobs to check if these errors appear with the If so, I would say that there is no much left we can do to solve this issue, just to wait until |
FYI I have opened numpy/numpy#16692 to see whether that was an intended change from numpy. |
Just for completeness the thing that is not picklable in numpy development version is |
You are absolutely right. Thanks for the correction! |
…arn#17707) * MNT More replacements of numpy aliases with built-in types [scipy-dev] * MNT More (manual) replacements of numpy aliases * MNT More (manual) replacements of numpy aliases * FIX Minor change * MNT Trigger build [scipy-dev] * FIX Minor change * MNT Trigger build [scipy-dev] * FIX More deprecations of numpy aliases [scipy-dev] * MNT Silent numpy aliases warnings [scipy-dev] * FIX Fix silent deprecations * Trigger build [scipy-dev] * FIX Add scape characters [scipy-dev] * FIX Remove package specification from silent * Trigger build [scipy-dev] * Revert
…arn#17707) * MNT More replacements of numpy aliases with built-in types [scipy-dev] * MNT More (manual) replacements of numpy aliases * MNT More (manual) replacements of numpy aliases * FIX Minor change * MNT Trigger build [scipy-dev] * FIX Minor change * MNT Trigger build [scipy-dev] * FIX More deprecations of numpy aliases [scipy-dev] * MNT Silent numpy aliases warnings [scipy-dev] * FIX Fix silent deprecations * Trigger build [scipy-dev] * FIX Add scape characters [scipy-dev] * FIX Remove package specification from silent * Trigger build [scipy-dev] * Revert
Reference Issues/PRs
Coming from #17687.
What does this implement/fix? Explain your changes.
This PR continues the deprecations of
numpy
aliases by built-in types.Any other comments?
This PR was generated by:
$ find . -name "*.py" -print0 | xargs -0 sed -i "s/\bnp.object\b/object/g"
Two manual changes were applied in
sklearn/ensemble/_gradient_boosting.pyx
andsklearn/neural_network/tests/test_rbm.py
.CC @thomasjpfan @glemaitre @adrinjalali