Skip to content

Fix EnumSerializer.write to accept numpy scalars from NDArray of records#285

Open
SAY-5 wants to merge 1 commit into
microsoft:mainfrom
SAY-5:say5-fix-enum-numpy-ndarray
Open

Fix EnumSerializer.write to accept numpy scalars from NDArray of records#285
SAY-5 wants to merge 1 commit into
microsoft:mainfrom
SAY-5:say5-fix-enum-numpy-ndarray

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented May 11, 2026

Closes #284.

EnumSerializer.write assumed its value argument was always a Python Enum, but when called from RecordSerializer._write on the numpy write path (where a record is iterated as value['fieldname'] from a np.void scalar), it receives a bare np.integer and crashes with AttributeError: 'numpy.uint64' object has no attribute 'value'. This affected any T[] whose element type was a record containing one or more enum fields.

Fix unwraps .value only when value is an Enum; numpy scalars (which _integer_serializer.write already accepts) are passed through. Adds python/tests/test_enum_in_ndarray.py covering: the original NDArray-of-record-with-enums reproducer, the bare numpy-scalar path directly through EnumSerializer.write, and the existing Python-Enum path (to guard against regression).

@SAY-5 SAY-5 force-pushed the say5-fix-enum-numpy-ndarray branch from 2a9f25d to 6f48815 Compare May 11, 2026 08:41
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not the right way to test this.

The test model should be updated such that it reproduces the issue. Then regression tests can be added to test_generated_types.py and test_protocol_roundtrip.py. Then, an equivalent test should be added for C++ and MATLAB to ensure no similar bugs exist.

@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented May 11, 2026

Understood, that's a much bigger refactor than I have bandwidth for; happy to close in favour of a more thorough fix.

@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented May 11, 2026

Understood, the proper test should reproduce the bug through the YAML test model and have regression coverage in the generated-types and protocol-roundtrip suites for Python, C++, and MATLAB. That's a substantially different scope from this PR, so I'll close this and open a follow-up once the test model and codegen changes are ready.

@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented May 11, 2026

Closing per maintainer feedback, will resubmit with the proper test model approach.

@SAY-5 SAY-5 closed this May 11, 2026
@SAY-5 SAY-5 reopened this May 12, 2026
@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented May 12, 2026

Reopened. Reworking per your feedback: I'll update the test model to reproduce the issue, add regression tests in test_generated_types.py and test_protocol_roundtrip.py, and add equivalent coverage for C++ and MATLAB to confirm no similar bugs exist there. Will push and re-ping.

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.

Python EnumSerializer raises AttributeError on NDArray of Record types containing enum fields

2 participants