-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[MRG+1] Fix the behavior of astype copy=True #4555
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
@@ -103,7 +103,7 @@ def divide(x1, x2, out=None, dtype=None): | |||
except TypeError: | |||
# Compat where astype accepted no copy argument |
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.
Does travis contain the old version?
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 seems it does.
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 think so. I don't have a numpy 1.6.1 at hand right now to quickly check.
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 did the check on numpy 1.6.1 with docker. numpy 1.6.1 is the version of numpy shipped with Ubuntu 12.04 and that is the oldest version of numpy supported by sklearn) and I got:
>>> a.astype(np.float32, copy=False)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: astype() takes no keyword arguments
we have a travis config for that. So this case is properly tested.
Apart from my comment LGTM. |
91fac5a
to
95fb92b
Compare
@amueller I forced pushed a change to use a |
Cool. LGTM. |
@jnothman any time for a quick review on this tiny tiny fix on your code? |
Random ping to get this fix in @arjoly @glouppe @mblondel @pprett @agramfort @GaelVaroquaux. |
LGTM! Thanks for the fix :s |
[MRG] Fix the behavior of astype copy=True
Thanks :) |
The astype backport should copy memory by default to be consistent with the default behavior of recent numpy.