-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: DataFrame.at with non-unique axes #33047
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
Conversation
@property | ||
def _axes_are_unique(self) -> bool: | ||
# Only relevant for self.ndim == 2 | ||
return self.obj.index.is_unique and self.obj.columns.is_unique |
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 this assertion explicity
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.
updated+green
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.
(need to discuss the desired behaviour)
i think consensus on the call was to move forward with this? |
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.
yes we discussed moving forward here. @jorisvandenbossche recollection correct?
gentle ping |
Yes, as also summarized on the issue (#33153) |
def __getitem__(self, key): | ||
if self.ndim == 2 and not self._axes_are_unique: | ||
# GH#33041 fall back to .loc | ||
return self.obj.loc[key] |
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 problem with this, though, is that it allows any kind of indexer for this case (not only scalar ones), and thus relaxing the requirements for .at
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.
Updated to address this issue
gentle ping |
Thanks! (BTW, if you have 2 approvals and no others commenting/requesting changes, I think it is fine to merge yourself) |
Good rule of thumb, thanks |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
Closes #33041 in conjunction with #33045.