Skip to content

BUG: Preserve sequence name during fancy indexing for Series subclasses with custom _metadata (#61491) - #66440

Open
pranavchoudhary-tech wants to merge 4 commits into
pandas-dev:mainfrom
pranavchoudhary-tech:fix-metadata-name-loss-61491
Open

BUG: Preserve sequence name during fancy indexing for Series subclasses with custom _metadata (#61491)#66440
pranavchoudhary-tech wants to merge 4 commits into
pandas-dev:mainfrom
pranavchoudhary-tech:fix-metadata-name-loss-61491

Conversation

@pranavchoudhary-tech

Copy link
Copy Markdown
Contributor

This PR fixes a bug where list-indexing (and methods like drop_duplicates) on a Series subclass would lose the sequence name if the subclass explicitly defined its own _metadata.
The root cause is that subclassing with custom _metadata completely overwrites Series._metadata = ["_name"], causing __finalize__ to skip propagating the _name attribute. This PR unconditionally propagates _name for 1-dimensional Series-like objects during __finalize__ to prevent this.

@aaron-seq

Copy link
Copy Markdown

Thanks for tracking down the root cause here - the reproduction and the diff are easy to follow. A few things I'd want addressed/discussed before merge.

Validated: CI is fully green (Unit Tests, Doc Build, Code Checks, Wheel builder, zizmor all pass), plus the fix logic, test coverage, and this against contributing_codebase.rst and extending.rst.

Business context / approach: issue #61491 is still labeled "Needs Triage" - no maintainer has confirmed a core-code fix (vs. a documentation fix) is the right direction. Per extending.rst, the documented pattern for subclasses is to extend _metadata (e.g. _metadata = Series._metadata + ["prop"]), not overwrite it; overwriting is what drops the inherited _name entry, which is the actual root cause. This PR instead adds an unconditional _name propagation step inside finalize (pandas/core/generic.py lines 6282-6283), which runs on nearly every Series operation, to compensate for subclasses that don't follow that convention. That's a broad, always-on change to a hot path for a narrow, arguably convention-violation case - the issue reporter's own suggestion was a documentation clarification instead. Could we get a maintainer's read on whether this belongs in finalize at all, versus documenting the convention or catching it at subclass-definition time?

Blocking: no whatsnew entry. Per contributing_codebase.rst, bug fixes need a release-note entry in doc/source/whatsnew/vX.Y.Z.rst referencing #61491 - none is included in this diff.

Non-blocking suggestions:
pandas/core/generic.py lines 6282-6283: worth a short comment (or test) confirming this doesn't clobber a name that was deliberately set earlier in the same operation before finalize runs (e.g. name-combination logic in binary ops), since that also flows through this method.

pandas/tests/series/test_subclass.py lines 93-98: the new test only checks .name survival; it doesn't set/assert seq.property, so there's no regression check that the existing _metadata propagation loop and the new unconditional _name propagation both work correctly together on the same object.

Minor (style): pandas/core/generic.py lines 6280-6281, the issue reference style "(GH61491)" doesn't match this file's usual "GH#61491" / "GH 13473" convention used elsewhere. pandas/tests/series/test_subclass.py line 94, this file's existing convention is a full issue URL in comments (see line 80); "# GH 61491" is fine but inconsistent with the local file style.

Given the above, I'd lean toward "comment" rather than approve/request changes at this stage - the change is small and passes CI, but I'd like maintainer input on the finalize approach and the missing whatsnew entry addressed before this is merge-ready.

(Drafted with AI assistance; I've reviewed and agree with these comments.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: If you add _metadata to a custom subclass of Series, the sequence name is lost when indexing

2 participants