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

Add Uint8ClampedArray to TypedArray_to_dtype mapping #94

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

zengspr
Copy link
Contributor

@zengspr zengspr commented Feb 17, 2025

When calling create_dataset with data that is a Uint8ClampedArray, the library throws the error "DataView not supported directly for write".

This appears to be because the TypedArray_to_dtype is missing a mapping for that array type, even though it is a permissible array type based on the GuessableDataTypes and TypedArray types.

This PR updates the mapping so Uint8ClampedArrays can be provided as the data parameter to create_dataset.

@bmaranville
Copy link
Member

Thanks for the PR! This one looks pretty straightforward.

@Carnageous
Copy link

Carnageous commented Feb 18, 2025

Just looking at the TypedArray_to_dtype map, Float16Array also seems to be missing here. Not sure if that is even supported though (It's not defined in TypedArray or TypedArrayConstructor)

@bmaranville
Copy link
Member

Just looking at the TypedArray_to_dtype map, Float16Array also seems to be missing here. Not sure if that is even supported though (It's not defined in TypedArray or TypedArrayConstructor)

Do people want support for Float16 in h5wasm for reading and/or writting? We could enable type conversion on read/write from a native type like Float32Array, or add an inefficient (compared to builtin TypedArrays) class for holding Float16 values in Javascript. Maybe a new issue if this is important?

@Carnageous
Copy link

I personally don't need it and I'm unsure if there are any use-cases. I just mentioned it because it is a built-in type in many browser engines. But you are right that it probably doesn't make a whole lot of sense in the context of hdf5 files.

@bmaranville
Copy link
Member

Wow - the support for Float16Array is weirdly distributed among browsers and server runtimes:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Float16Array#browser_compatibility

@bmaranville bmaranville merged commit 0c42d74 into usnistgov:main Feb 18, 2025
1 check passed
@axelboc
Copy link
Collaborator

axelboc commented Feb 19, 2025

Yeah, Float16Array is out now. It'd definitely be worth accepting/returning Float16Array on browsers that support it if possible.

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.

4 participants