Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDF5: Fix String Vlen Attribute Reads #1084

Merged
merged 1 commit into from
Aug 11, 2021
Merged

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Aug 10, 2021

We unofficially try to also support HDF5 variable lengths strings in reading, just to increase compatibility.
The standard mandates fixed-lengths strings, so we only add this in reads.

This was never working it seems.

We inofficially try to also support HDF5 variable lengths strings in
reading, just to increase compatibility.

This was never working it seems.
@ax3l
Copy link
Member Author

ax3l commented Aug 11, 2021

I dunno how to add an automated test for this besides writing something with h5py manually...

@ax3l
Copy link
Member Author

ax3l commented Aug 11, 2021

Since I tested this locally, I will merge it in to fix the existing bug.

@ax3l ax3l merged commit 1b29cb6 into openPMD:dev Aug 11, 2021
@ax3l ax3l deleted the fix-vlenStringsHDF5 branch August 11, 2021 06:47
@ax3l ax3l added this to the 0.14.2 milestone Aug 11, 2021
ax3l added a commit to ax3l/openPMD-api that referenced this pull request Aug 18, 2021
We inofficially try to also support HDF5 variable lengths strings in
reading, just to increase compatibility.

This was never working it seems.
ax3l added a commit that referenced this pull request Aug 18, 2021
* Bug fix: Don't forget closing files (#1083)

* Failing test
* Bug fix: Don't forget closing files

* HDF5: Fix String Vlen Attribute Reads (#1084)

We inofficially try to also support HDF5 variable lengths strings in
reading, just to increase compatibility.

This was never working it seems.

* Fix reading of vector attributes with only one contained value (#1085)

* Failing test

* Conversions in Attribute.hpp

1) single values to 1-value vectors
2) vectors to arrays
3) arrays to vectors

* Some cleanup in Attribute.hpp

1) Simplify types in DoConvert, remove unnecessary template parameter
2) Replace a long if-then-else chain by variantSrc::visit

* CoreTest: Fix std::array constructors

Make more widely compile-able.

* Explicit casting in some places

This avoids some warnings

* Intel compilers: Don't use variantSrc::visit

They don't like it

* Remove scattered checks for vector attributes

* Generalize icpc guard

As defined in CMake for compiler identification.

* Doc ICC version (2021.3.0)

* setAttribute: Reject Empty Strings (#1087)

* setAttribute: Reject Empty Strings

Some backends, especially HDF5, do not allow us to define zero-sized
strings. We thus need to catch this in the frontend and forward the
restriction to the user.

* Test: setAttribute("key", "") throws

* setAttribute Check: C++14 compatible

* Don't read iterations if they have already been parsed (#1089)

* Release: 0.14.2

Co-authored-by: Franz Pöschel <franz.poeschel@gmail.com>
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.

1 participant