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

Fetch opaque datasets with h5grove as binary #1587

Merged
merged 3 commits into from
Mar 13, 2024
Merged

Fetch opaque datasets with h5grove as binary #1587

merged 3 commits into from
Mar 13, 2024

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented Mar 7, 2024

This change is in preparation for #1554.

Opaque datasets were so far fetched with h5grove as JSON, which led to back-end errors for datasets that cannot be encoded to strings (like datetime64_scalar).

I now fetch opaque datasets as binary and return UInt8Array values from H5GroveProvider for consistency with H5WasmProvider.

With h5grove@2.0.0, this change leads to 422 errors for all opaque datasets. This will be fixed with silx-kit/h5grove#89 and a new release of h5grove. This PR therefore constitutes somewhat a breaking change.

While I'm at it, I also improve the display of UInt8Array values slightly in RawVis by simply calling toString() instead of JSON.stringify():

image image

@axelboc
Copy link
Contributor Author

axelboc commented Mar 11, 2024

I've updated the h5grove demo back-end following the merge of silx-kit/h5grove#89. As planned, we no longer get 422 errors when fetching opaque datasets and the values for those datasets in h5grove-api.test.ts.snap now match the values in h5wasm-api.test.ts.snap. 🎉

@axelboc axelboc marked this pull request as ready for review March 11, 2024 13:20
@axelboc axelboc requested a review from loichuder March 11, 2024 13:22
@axelboc axelboc merged commit 0febbd9 into main Mar 13, 2024
8 checks passed
@axelboc axelboc deleted the bin-opaque branch March 13, 2024 09:19
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.

2 participants