DEPR: Raise FutureWarning about raising an error in __array__ when copy=False cannot be honored
#60395
Conversation
KevsterAmp
commented
Nov 22, 2024
•
KevsterAmp
commented
- closes DEPR: deprecate / warn about raising an error in __array__ when copy=False cannot be honore #60340(Replace xxxx with the GitHub issue number)
- Tests added and passed if fixing a bug or adding a new feature
- All code checks passed.
pandas/core/arrays/arrow/array.py
Outdated
| import warnings | ||
|
|
||
| from pandas.util._exceptions import find_stack_level |
There was a problem hiding this comment.
You can move those warnings to the top of the file
pandas/core/arrays/arrow/array.py
Outdated
| "raise an error when a zero-copy numpy array is not possible", | ||
| FutureWarning, | ||
| stacklevel=find_stack_level(), | ||
| ) | ||
| # TODO: By using `zero_copy_only` it may be possible to implement this | ||
| raise ValueError( |
There was a problem hiding this comment.
This error should then be removed (we are adding the warning instead of the error, to warn people it will error in the future).
There was a problem hiding this comment.
Gotcha, I initially thought we would raise both an error and a warning.
pandas/core/arrays/categorical.py
Outdated
| import warnings | ||
|
|
||
| from pandas.util._exceptions import find_stack_level |
There was a problem hiding this comment.
Same here about the imports (and similar for next cases)
pandas/core/arrays/categorical.py
Outdated
| "raise an error when a zero-copy numpy array is not possible", | ||
| FutureWarning, | ||
| stacklevel=find_stack_level(), | ||
| ) | ||
| raise ValueError( |
There was a problem hiding this comment.
And same for the errors
jorisvandenbossche
added
Compat
|
Thanks for the review despite this PR being in draft. Working on the tests and changes based on the comments, will mark it as ready once done |
…lse-futurewarning
|
Re-marking this as draft since I'll be fixing the errors on CI |
| with pytest.raises(ValueError, match="Unable to avoid copy while creating"): | ||
| msg = "Starting on NumPy 2.0, the behavior of the 'copy' keyword has changed " | ||
| "and passing 'copy=False' raises an error when a zero-copy NumPy array " | ||
| "is not possible, Pandas will follow this behavior starting with " | ||
| "version 3.0. This conversion to NumPy requires a copy, but " | ||
| "'copy=False' was passed. Consider using 'np.asarray(..)' instead." | ||
| with tm.assert_produces_warning(FutureWarning, match=msg): |
There was a problem hiding this comment.
It's fine to just test a small part of the message, just so we assert the correct message is raised. Something like
msg = "Pandas will follow this behavior starting with version 3.0"
with tm.assert_produces_warning(FutureWarning, match=msg):
...
There was a problem hiding this comment.
And for the other CI errors, essentially you will have to replace all occurrences of with pytest.raises(ValueError, match="Unable to avoid copy while creating") with this assert_produces_warning
There was a problem hiding this comment.
Thanks, will work on this in a couple of days. Been too focused on AoC this December 😆
There was a problem hiding this comment.
is it alright if I match the first part of the message?
msg = "Starting with NumPy 2.0, the behavior of the 'copy' keyword"
with tm.assert_produces_warning(FutureWarning, match=msg):
np.array(arr, copy=False)Using
msg = "Pandas will follow this behavior starting with version 3.0"is failing
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
|
@KevsterAmp do you have time to update this? |
…lse-futurewarning
