ENH: enable Py_TPFLAGS_SEQUENCE for the ndarray object
#30653
Merged
+237
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
I have added testing for ndarray and masked array. Please let me know what more do you want tested here (other subclasses for example) and whether we can do testing against the wider ecosystem to ensure this doesn't break anything. I was lazy so I had claude write me this: import numpy as np
import tempfile
import os
# 1. ndarray (base)
print("=== ndarray ===")
arr = np.array([1, 2, 3])
match arr:
case [a, b, c]:
print(f"PASS: matched {a}, {b}, {c}")
case _:
print("FAIL")
# 2. MaskedArray
print("\n=== MaskedArray ===")
arr = np.ma.array([1, 2, 3], mask=[0, 1, 0])
match arr:
case [a, b, c]:
print(f"PASS: matched {a}, {b is np.ma.masked}, {c}")
case _:
print("FAIL")
# 3. recarray
print("\n=== recarray ===")
dt = np.dtype([('name', 'U10'), ('age', 'i4')])
arr = np.array([('Alice', 25), ('Bob', 30)], dtype=dt).view(np.recarray)
match arr:
case [first, second]:
print(f"PASS: matched {first.name}={first.age}, {second.name}={second.age}")
case _:
print("FAIL")
# 4. matrix (deprecated)
print("\n=== matrix ===")
with np.testing.suppress_warnings() as sup:
sup.filter(PendingDeprecationWarning)
arr = np.matrix([[1, 2], [3, 4]])
match arr:
case [row1, row2]:
print(f"PASS: matched {row1.tolist()}, {row2.tolist()}")
case _:
print("FAIL")
# 5. chararray (legacy)
print("\n=== chararray ===")
arr = np.char.array(['hello', 'world', 'test'])
match arr:
case [a, b, c]:
print(f"PASS: matched '{a}', '{b}', '{c}'")
case _:
print("FAIL")
# 6. memmap
print("\n=== memmap ===")
with tempfile.NamedTemporaryFile(delete=False) as f:
fname = f.name
arr = np.memmap(fname, dtype='float32', mode='w+', shape=(3,))
arr[:] = [1.0, 2.0, 3.0]
match arr:
case [a, b, c]:
print(f"PASS: matched {a}, {b}, {c}")
case _:
print("FAIL")
del arr
os.unlink(fname)they all fail without this PR while the output with this PR is |
Contributor
Author
|
There is a segfault in macos_arm64 - 3.14t parallel testing with trace below. It doesn't look related to this PR to me but I could be very wrong here. I just don't know |
Contributor
Author
|
Hmm unless the stack is wrong and the test that's actually crashing is |
Member
|
I'm also not able to reproduce the segfault. That said, I do see a data race inside CPython when I run Detailsbut I also see that same data race on numpy I'm going to push a commit so the test job prints more output. Let's see if it's just a random flake... |
Member
|
Seems to be a flaky failure. |
Contributor
Author
|
Yeah it passed this time. I've also been running |
Contributor
Author
|
Alright so, I was considering adding pattern matching tests here for all the ndarray subclasses that numpy makes. Should I proceed with that here? |
|
I don't think the stacktrace is wrong, but it's very possible that the bug or memory corruption occurs earlier than the crash. The C stacktrace ends with
The |
|
Also, I suspect that the data race seen by @ngoldbaum wasn't the cause of the crash, but it's hard to know for sure. |
Member
|
Thanks for taking a look, Sam! I'll keep an eye out for this and perhaps try running with an ASan build to see if that spots anything.
Skip chararray, that's in the process of being deprecated: #30605. So I guess that just leaves memmap and recarray. Is there another one I'm missing? |
Contributor
Author
|
|
Member
|
Ah yes, matrix too, but IMO that's skippable (see e.g. #13835 for why it's unloved). Even doing more than masked array is probably overkill. |
Contributor
Author
|
More tests added. I sleep better if I test the other subclasses too. Matrix is “interesting” because it has peculiar pattern matching in the sense that it is always 2D. The test failure is unrelated (it failed during adding deadsnakes ppa) |
ngoldbaum
reviewed
Jan 15, 2026
Member
ngoldbaum
left a comment
There was a problem hiding this comment.
Overall looks great.
I think we should probably add a release note for the new ability to do structural pattern matching perhaps including a code example. I don't know if it needs to be explicitly documented, but maybe look over the docs for a spot to add a short explanation of how users can pattern match with arrays, and then you can link to the new docs from the release note.
There's a README in the root of the doc/release folder that explains how to write release notes.
Contributor
Author
|
I have added release notes and docs (hopefully correctly). I thought that adding a small section at the bottom of this https://numpy.org/devdocs/reference/arrays.ndarray.html was a good place to do so. Let me know if you want it more visible like maybe in the user guide too. |
Contributor
Author
|
Should I also revert Nathan's spin test verbosity change or do you wanna keep that for the future? (it may be useful although bit noisy in the log) |
Member
Let's keep it, it would have made debugging the crash you ran into a bit easier. |
ngoldbaum
approved these changes
Jan 16, 2026
Member
ngoldbaum
left a comment
There was a problem hiding this comment.
Looks good to me! I have one small suggestion for the docs.
I'm going to hold off on merging this because it hasn't been open for very long and it's a new feature. I doubt anyone will object but I like to give time for for other maintainers to raise objections on ENH PRs.
Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
Member
|
No comments since last week: let's merge this. Thanks @ikrommyd! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #30519
I now get this locally which didn't match before
I'm putting this up to see what CI says first