Handle different behavior of .array() in NumPy 2.0.0 #3752
Conversation
|
@mlpack-jenkins test this please |
jakirkham
left a comment
jakirkham
left a comment
There was a problem hiding this comment.
Thanks Ryan! 🙏
Had a question below
| if copy: | ||
| out = np.asarray(x, dtype=dtype, copy=True) | ||
| else: | ||
| out = np.asarray(x, dtype=dtype, copy=None) |
There was a problem hiding this comment.
Don't think asarray had the copy argument in NumPy 1. It isn't listed in the NumPy 1 docs. Also NumPy 2 docs note copy is a new argument. Testing local with NumPy 1 am seeing an error. Am I missing something? Or are these meant to be array instead?
There was a problem hiding this comment.
Ahh, thank you! I did mean to write array() there, that was the documentation I checked to come up with this fix. Just pushed a fix now.
| if copy: | ||
| out = np.array(x, dtype=dtype, copy=True) | ||
| else: | ||
| out = np.array(x, dtype=dtype, copy=None) |
There was a problem hiding this comment.
Think the None option is new in NumPy 2. It isn't mentioned in NumPy 1. So likely there is another condition needed to check for NumPy 2 and use the copy option then and if not drop the copy argument. That is assuming that is the behavior intended here
There was a problem hiding this comment.
Thanks! I thought I could get away with it (and was depending on CI to tell me if I was wrong, which it did), but you're right. Should be fixed in 57d2e8b.
|
Thanks all! 🙏 |
4 participants
This should fix #3723; the behavior of
np.array()changed in NumPy 2.0.0 and now throws aValueErrorif thecopy=Falseargument is given and a copy cannot be avoided. But we were usingcopy=Falsewith the intention of "don't copy unless you need to"---where the more correct option is just to passNoneinstead. According to the documentation that should be backwards-compatible with NumPy 1.x too; CI will tell us if that's true in just a few minutes. :)@jakirkham if you happen to have a moment to review, it would be appreciated, but no worries if too much is going on. 👍