-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DEPR: Silent dropping of nuisance columns in agg_list_like #43741
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
Merged
jreback
merged 11 commits into
pandas-dev:master
from
rhshadrach:depr_fallback_agg_dict
Sep 29, 2021
Merged
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
bc67672
DEPR: Dropping of silent columns in NDFrame.agg with list-like func
rhshadrach a1f7277
DEPR: Dropping of silent columns in NDFrame.agg with list-like func
rhshadrach e0fddd8
Merge branch 'master' of https://github.com/pandas-dev/pandas into de…
rhshadrach bbece5e
Remove check_stacklevel=False
rhshadrach f08c782
Minor refactor
rhshadrach 5e9de2d
Fixup for whatsnew
rhshadrach 816db57
Doc fixups
rhshadrach 9f47591
Pass through user warnings, update whatsnew
rhshadrach 38eb867
Merge branch 'master' of https://github.com/pandas-dev/pandas into de…
rhshadrach 78e0133
Supress warning in docs
rhshadrach 450795f
Merge branch 'master' of https://github.com/pandas-dev/pandas into de…
rhshadrach File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
are all warnings that happen here going to represent a failure?
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.
Woah - thanks. No, probably not. Will inspect the warnings themselves.
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 got a bit hairy, I wasn't able to find a good way to capture and suppress only our warnings while passing through any other warnings and being able to determine if we raised. Supplying a filter within the catch_warnings context manager means we can't fell if we raised; using record=True there means all user warnings would also be suppressed.
If there are any suggestions for a better implementation, it would be appreciated.
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.
what happens if you dont catch warnings at all here?
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.
Running
gives
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 there's something like a
filter="once"
to prevent exactly that (and i think its the default)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 see - thanks. Digging into this, the only way I can see why I'm getting multiple warnings is because of https://bugs.python.org/issue29672. I haven't confirmed yet if the code within agg ever hits a separate
catch_warnings
; if it does that will cause repeat messages.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'm still going to verify that the use of another catch_warnings (in a different part of the code) is the cause for the multiple messages I am seeing without the special handling in this PR.
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've confirmed it is because of the Python bug mentioned above. The code in the sample hits
pandas.core.indexes.base.Index_with_infer
which uses the catch_warnings context manager. Here is a MWE to demonstrate the bug (mirroring the structure ofagg_list_like
):With the call to
foo
from withinbar
, three warning messages are printed. When the call tofoo
is removed, only one is printed.With this, are we okay with catching the warnings as was merged @jbrockmendel?
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.
im fine with it. a comment to this effect might be worthwhile