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.
BUG: Multiindexed series .at fix #32520
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
BUG: Multiindexed series .at fix #32520
Changes from all commits
083b1ec
88bca04
f325e09
d03b102
bf4f1c4
89ee128
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Can you add a test for not providing all MultiIndex levels? (so eg
series.at[1]
).And also a test with a duplicated label?
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 maybe also an equivalent test for a DataFrame (it seems to work, but would be good to ensure we also have an explicit test for that)
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.
Sure - I have it mostly done, but ran into a question. For the series
i.e.
what should be the output of series.at[1, 3]? I was expecting to just get the scalar 1, but currently I'm getting
After thinking about this - perhaps what we have currently is the desired output. My reason for this is that the shape doesn't change depending on the input - (1, 3) vs (2, 4).
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.
IMO, that should be a scalar (but that is also the reason that I asked to test it, to be sure this is covered ànd to discuss the semantics we want)
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.
is this the behavior now on master? IOW is this an api change
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.
Current behavior on master is to raise ValueError for any of the at lines, behavior with the loc lines is the same as master. I would describe this as a bugfix and not an api change.