-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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: fix stacklevel for DataFrame(mgr) deprecation #55591
DEPR: fix stacklevel for DataFrame(mgr) deprecation #55591
Conversation
|
This reverts commit 4c15c48.
msg = "Passing a BlockManager to DataFrame is deprecated" | ||
with tm.assert_produces_warning(DeprecationWarning, match=msg): | ||
result = table.to_pandas() | ||
result = table.to_pandas() |
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.
In several of those cases I removed the warning assert, and added a general filterwarnings mark (at the test or top of the file), because 1) there is no need to "test" that the warning is raised in all those cases, and 2) once the PR to fix this on the pyarrow side is merged, all those cases will start failing anyway
As mentioned on the issue, I updated this to use And the tests are all passing now. |
Any other comments here? |
pandas/core/frame.py
Outdated
@@ -697,7 +697,7 @@ def __init__( | |||
"is deprecated and will raise in a future version. " | |||
"Use public APIs instead.", | |||
DeprecationWarning, | |||
stacklevel=find_stack_level(), | |||
stacklevel=1, # bump to 2 once pyarrow is released with fix |
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.
Nit: Could you reference the anticipated pyarrow version that would have this fix (if the fix has already been made in pyarrow)?
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.
That's not yet fully sure. At least 15.0.0 I assume, but potentially a 14.0.x release as well. But will add 15.0 then.
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.
One small request otherwise LGTM
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.
Merge conflict otherwise LGTM
Follow-up on #52419. To ensure end users don't see this warning (unless they would be passing a manager themselves), but only libraries see them in their tests.
See #52419 (comment) for context.