Skip to content

ENH: Improvements and fixes for stringdtype hashing - #32563

Merged
ngoldbaum merged 3 commits into
numpy:mainfrom
ngoldbaum:fix-stringdtype-hashing
Sep 16, 2026
Merged

ngoldbaum merged 3 commits into
numpy:mainfrom
ngoldbaum:fix-stringdtype-hashing

Conversation

@ngoldbaum

@ngoldbaum ngoldbaum commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

PR summary

This fixes some issues with hashing of StringDType objects of a na_object is defined.

First, the hashing implementation in both tp_hash and the np.unique hash-based unique implementation did not properly take into account all kinds of missing data supported by StringDType. These could lead to inconsistent behavior. The former in more widespread situations: for example unpickling a dtype instance with na_object=np.nan would produce an object that compares equal but with a distinct hash because the unpickled object has a distinct nan instance.

The latter would lead to similar inconsistencies and duplicated values in np.unique. It also makes np.unique(..., equal_nan=True) do the right thing for nan-like sentinels.

AI Disclosure

I used an AI model to iterate on this.

@ngoldbaum
ngoldbaum force-pushed the fix-stringdtype-hashing branch from e96af16 to e118703 Compare September 9, 2026 23:18
@ngoldbaum ngoldbaum added the triage review Issue/PR to be discussed at the next triage meeting label Sep 15, 2026

@seberg seberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I am fine with this (would be nice to tweak the release note a bit).

Maybe it makes sense to backport the first commit?

Heh, to me the second one (unique fix) is the more obvious to backport as it seems like a very clear bug fix :).

For the NaN-hash problem there is just no way to avoid some paradoxes... (since Python doesn't resolve them either)

__eq__ makes all NaNs (that are floats or float subclasses) equal, but of course the actual object for arr[na_index] may not be :).

In an ideal world an NA object that isn't a string would always be a true singleton :(.
To some degree makes me wonder if na_object=NaN should just normalize to "our" NaN, but I doubt there is a path forward...

Either way... it doesn't matter, there is no perfect solution here so this is a best effort decision and I am fine with the direction here, even if it is neither (and can't be) complete, nor am I sure quite it is the only/best approach.

Comment thread doc/release/upcoming_changes/32563.improvement.rst Outdated
Comment thread numpy/_core/src/multiarray/stringdtype/dtype.c
@ngoldbaum ngoldbaum added the 09 - Backport-Candidate PRs tagged should be backported label Sep 16, 2026
@ngoldbaum

Copy link
Copy Markdown
Member Author

Heh, to me the second one (unique fix) is the more obvious to backport as it seems like a very clear bug fix :).

OK, let's mark the whole thing as a backport. It applies cleanly and fixes some straight-up bugs!

I'll merge this once CI passes, thanks for the review!

@ngoldbaum ngoldbaum removed the triage review Issue/PR to be discussed at the next triage meeting label Sep 16, 2026
@ngoldbaum
ngoldbaum merged commit 60269b1 into numpy:main Sep 16, 2026
91 checks passed
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Sep 20, 2026
charris added a commit that referenced this pull request Sep 20, 2026
ENH: Improvements and fixes for stringdtype hashing (#32563)
antareepsarkar pushed a commit to antareepsarkar/numpy that referenced this pull request Oct 5, 2026
@nunyaMFbusinessbot

This comment has been minimized.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants