Skip to content

ENH: fixes for StringDType sorting and comparisons for missing data - #32564

Merged
ngoldbaum merged 5 commits into
numpy:mainfrom
ngoldbaum:fix-nullable-comparisons
Sep 17, 2026
Merged

ngoldbaum merged 5 commits into
numpy:mainfrom
ngoldbaum:fix-nullable-comparisons

Conversation

@ngoldbaum

@ngoldbaum ngoldbaum commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

PR summary

Fixes a couple bugs in the StringDType implementation, where missing data doesn't properly implement equality operators.

For nan-like missing data, it's the != operator:

>>> import numpy as np
>>> arr = np.array(["hello", np.nan, "world"], dtype=np.dtypes.StringDType(na_object=np.nan))
>>> arr == "hello"
array([ True, False, False])
>>> arr != "hello"
array([False, False,  True])

The answer for the last expression should be array([False, True, True]). The current answer is a straight-up bug. Unfortunately it also changes operations like arr[data != sentinel], so we should probably not backport it.

This also fixes this issue:

>>> arr = np.array([None, "", "hello"], dtype=np.dtypes.StringDType(na_object=None))
>>> arr == ""
array([ True,  True, False])
>>> arr != ""
array([False, False,  True])

NumPy says None == "" here! Under this PR, you get:

>>> arr == ""
array([False,  True, False])
>>> arr != ""
array([ True, False,  True])

As a result of these two fixes, also fixes issues with the np.unique code paths that use the sort-based implementation, see the tests.

This also includes a bugfix for numpy's testing utilities. At some point I added a second implementation of StringDType's missing value classification, which is broken in a way that the tests added here expose. The fix is to defer to the rules the dtype actually uses.

AI Disclosure

I iterated with an AI on this.

@ngoldbaum ngoldbaum added 00 - Bug component: numpy.strings String dtypes and functions labels Sep 9, 2026
@ngoldbaum ngoldbaum changed the title ENH: fixes for StringDType sorting and comparison missing data ENH: fixes for StringDType sorting and comparisons for missing data Sep 9, 2026
@ngoldbaum
ngoldbaum force-pushed the fix-nullable-comparisons branch from 710ba0a to ac85d11 Compare September 9, 2026 23:39
@ngoldbaum ngoldbaum added the triage review Issue/PR to be discussed at the next triage meeting label Sep 15, 2026
@ngoldbaum
ngoldbaum force-pushed the fix-nullable-comparisons branch from ac85d11 to 18f077d Compare September 16, 2026 21:42
@ngoldbaum

Copy link
Copy Markdown
Member Author

@seberg any chance I can get you to look at this one as well? I just pushed a commit that cuts down the new tests quite a bit. Most of this is still new tests, the actual functional changes are small and I think the comparison fix is clearly correct.

I have a nice followup for this that makes np.isin ~100x faster for StringDType data. A Quansight client noticed the slowdown in production.

My fix relies entirely on StringDType's own hashing and sorting implementation. However making it work correctly required merging this and #32563.

@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, are you sure about backporting? I am wondering if we are getting too aggressive, again; but fine if this is a real world issue. (Not even sure Chuck has generated release-notes for those in the past, but that can be done.)

The other thing I'll note is that (pd.NA != object()) is pd.NA, so for that this is all a bit unclear if it is defined at all.
That said, I guess it is still the better/typically expected behavior.

I do insist on that test, though!

Comment thread numpy/_core/tests/test_stringdtype.py Outdated
assert_array_equal(left == right, expected)
assert_array_equal(left != right, ~expected)
assert_array_equal(left == "", [True, False, False, False, True, False])
assert_array_equal("" != left, [False, True, True, True, False, True])

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.

I'm sure it's correctly used in the comparison ufunc, but please expand this to all comparisons.

@ngoldbaum
ngoldbaum force-pushed the fix-nullable-comparisons branch from 5f0c3d2 to 5fc53d7 Compare September 17, 2026 19:41
@ngoldbaum ngoldbaum removed the triage review Issue/PR to be discussed at the next triage meeting label Sep 17, 2026
@ngoldbaum
ngoldbaum merged commit 039f747 into numpy:main Sep 17, 2026
91 checks passed
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.

2 participants