Repository navigation
ENH: Improvements and fixes for stringdtype hashing - #32563
Conversation
e96af16 to
e118703
Compare
seberg
left a comment
There was a problem hiding this comment.
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.
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! |
ENH: Improvements and fixes for stringdtype hashing (#32563)
PR summary
This fixes some issues with hashing of StringDType objects of a
na_objectis defined.First, the hashing implementation in both
tp_hashand thenp.uniquehash-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 withna_object=np.nanwould produce an object that compares equal but with a distinct hash because the unpickled object has a distinctnaninstance.The latter would lead to similar inconsistencies and duplicated values in
np.unique. It also makesnp.unique(..., equal_nan=True)do the right thing for nan-like sentinels.AI Disclosure
I used an AI model to iterate on this.