BUG: ensure to always return new objects in Index set operations (avoid metadata mutation) #63174
Conversation
deniskrds
force-pushed
the
fix-index-setops-mutation
branch
from
88fecc0 to
3e9186d
Compare
rhshadrach
added
Bug
Index
rhshadrach
left a comment
rhshadrach
left a comment
There was a problem hiding this comment.
Looking good, would like a 2nd eye on this. cc @jbrockmendel
pandas/tests/indexes/test_setops.py
Outdated
| tm.assert_index_equal(res, expected) | ||
|
|
||
|
|
||
| class TestSetOpsMutation: |
There was a problem hiding this comment.
We'd like to move away from having test classes in cases where the class itself isn't useful. Can you just make these functions.
There was a problem hiding this comment.
Got it. I'll convert them to functions shortly.
jorisvandenbossche
left a comment
jorisvandenbossche
left a comment
There was a problem hiding this comment.
Looks good!
Could you add a note about this in the 3.0.0.rst whatsnew file? (eg in the indexing subsection in the bug fixes section)
pandas/core/indexes/base.py
Outdated
| if self.name is not name: | ||
| return self.rename(name) | ||
| return self | ||
| return self.copy() |
There was a problem hiding this comment.
| return self.copy() | |
| return self.copy(deep=False) |
I know that is the default, but for code readability I might add it explicitly (given the default for Series/DataFrame is True, one might expect that here as well; I was at least confused about when first looking at this)
| # Corner cases | ||
| inter = first.intersection(first, sort=sort) | ||
| assert inter is first | ||
| assert inter is not first |
There was a problem hiding this comment.
| assert inter is not first | |
| assert inter is not first | |
| tm.assert_index_equal(inter, first) |
Just to ensure it are still the same values
jorisvandenbossche
changed the title
|
Thanks for the review - all feedback addressed in the recent commits |
jorisvandenbossche
dismissed
rhshadrach’s stale review
comments are addressed
3 participants
…n) (#63169)
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.AGENTS.md.Previously,
Index.intersectionandIndex.union(includingMultiIndeximplementations) would return a direct reference toself(orother) if the result was identical to the input. This allowed side effects where mutating the metadata (e.g..name) of the result would corrupt the original index.This change forces a shallow copy (
deep=False) in these cases. This preserves the performance benefit of sharing the underlying data array while ensuring the Index container itself is a distinct object with independent metadata.