-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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
DEPR: Raise FutureWarning
about raising an error in __array__ when copy=False cannot be honored
#60395
base: 2.3.x
Are you sure you want to change the base?
DEPR: Raise FutureWarning
about raising an error in __array__ when copy=False cannot be honored
#60395
Conversation
KevsterAmp
commented
Nov 22, 2024
•
edited
Loading
edited
- 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.
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, looking good!
If you remove the error, you will have to update some tests that are now checking for an error (change that to check for a warning, with tm.assert_produces_warning
)
And see the issue for a suggestion to expand the message.
pandas/core/arrays/arrow/array.py
Outdated
import warnings | ||
|
||
from pandas.util._exceptions import find_stack_level |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And same for the errors
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 |