Fix complex scalar attribute read and add std::variant attribute support#41
Merged
Merged
Conversation
array_interface::write_attribute writes a std::complex scalar with rank 1 (trailing-2 FLOAT convention), but read_attribute hard-coded rank == 0 and threw on read. Match read_attribute's rank check to the view's rank. Assisted-by: Claude <noreply@anthropic.com>
Add h5_write_attribute / h5_read_attribute overloads for std::variant,
mirroring the existing dataset-side dispatch: write visits the active
alternative, read recovers it by matching the attribute's HDF5 datatype
against each variant member via hdf5_type_equal.
A small get_hdf5_type_attribute helper is added to object.{hpp,cpp} so
the templated variant header stays free of HDF5 headers.
Assisted-by: Claude <noreply@anthropic.com>
Replace the recursive detail::h5_read_variant_helper with an inline short-circuit fold over the variant alternatives, matching the form already used by the attribute-side h5_read_attribute. Assisted-by: Claude <noreply@anthropic.com>
lukas-weber
approved these changes
Jun 9, 2026
lukas-weber
left a comment
lukas-weber
left a comment
There was a problem hiding this comment.
Looks good to me, but if there is a simple way to get a string representation of the allowed types, it could be helpful to add to the error message.
Wentzell
force-pushed
the
attribute-variant-and-complex-fix
branch
from
a32008e to
76f281e
Compare
When h5_read / h5_read_attribute for std::variant fails to match the stored HDF5 datatype against any alternative, include the names of the allowed types (via the existing get_name_of_h5_type helper) in the thrown error message to make the diagnostic actionable. Assisted-by: Claude <noreply@anthropic.com>
Wentzell
force-pushed
the
attribute-variant-and-complex-fix
branch
from
76f281e to
f56c751
Compare
Contributor
|
Looks good! |
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
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.
Summary
Three small attribute-side improvements to the HDF5 wrappers:
std::complexscalar attribute read.array_interface::write_attributewrites complex scalars with rank 1 (trailing-2 FLOAT convention), butread_attributehard-codedrank == 0and threw on read. The rank check now matches the view's rank, making the round-trip symmetric.std::variantattribute support. Newh5_write_attribute/h5_read_attributeoverloads mirror the existing dataset-side dispatch: write visits the active alternative; read recovers it by matching the attribute's HDF5 datatype against each variant member viahdf5_type_equal. A smallget_hdf5_type_attributehelper is added toobject.{hpp,cpp}sostl/variant.hppstays free of HDF5 headers.h5_read_variant_helper. Replace the recursivedetail::h5_read_variant_helperin the dataset-sideh5_readwith an inline short-circuit fold over the variant alternatives, matching the form already used byh5_read_attribute. No behavior change.Tests in
test/c++/h5_complex.cppandtest/c++/h5_variant.cppwere extended accordingly.