Fix EnumSerializer.write to accept numpy scalars from NDArray of records#285
Fix EnumSerializer.write to accept numpy scalars from NDArray of records#285SAY-5 wants to merge 1 commit into
Conversation
Signed-off-by: SAY-5 <say.apm35@gmail.com>
2a9f25d to
6f48815
Compare
There was a problem hiding this comment.
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.
|
Understood, that's a much bigger refactor than I have bandwidth for; happy to close in favour of a more thorough fix. |
|
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. |
|
Closing per maintainer feedback, will resubmit with the proper test model approach. |
|
Reopened. Reworking per your feedback: I'll update the test model to reproduce the issue, add regression tests in |
Closes #284.
EnumSerializer.writeassumed itsvalueargument was always a PythonEnum, but when called fromRecordSerializer._writeon the numpy write path (where a record is iterated asvalue['fieldname']from anp.voidscalar), it receives a barenp.integerand crashes withAttributeError: 'numpy.uint64' object has no attribute 'value'. This affected anyT[]whose element type was a record containing one or more enum fields.Fix unwraps
.valueonly whenvalueis anEnum; numpy scalars (which_integer_serializer.writealready accepts) are passed through. Addspython/tests/test_enum_in_ndarray.pycovering: the original NDArray-of-record-with-enums reproducer, the bare numpy-scalar path directly throughEnumSerializer.write, and the existing Python-Enum path (to guard against regression).