h5wasm
h5wasm copied to clipboard
Add Typed Files/Groups
I want to contribute a feature I will likely need in the future: Adding types to Files/Groups in order to describe their content. For example, if an HDF5 file has some groups like "data" and "metadata", each containing some datasets, an interface for the File could look like this:
interface MyHDF5File {
data: {
dataset_1: Dataset;
subsets: {
dataset_3: Dataset;
}
},
metadata: {
metadata_1: Dataset;
}
}
With this, you could "strongly type" any file, and even get type hints in group.get()
methods. (with typescript Template Literals)
const file = h5wasm.File<MyHDF5File>(...)
file.get("/data/dataset_4") // results in typescript error, as it does not exist in the Interface.
First of all, would it okay to add such a feature? If so, I would like to implement it and open a PR. It would, of course, be optional and should not break any existing code.
While working with h5wasm locally, I noticed some things:
- Some tests seem to be failing. For
datatype_test.mjs
I get an error:
AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected
+ Datatype {
- {
type: 'Datatype'
}
- There is no proper description in the readme on how to build h5wasm. It's quite straight-forward, but adding a small step-by-step guide could be helpful (also naming the emscripten SDK as a dependency). If you want I can open a PR for that, though I am not very familiar with the emscripten/c ecosystem.
As a side-note, I saw in the git history that you thought about switching to esbuild for building h5wasm? Is there any help needed there?
Thank you!
I like the TypeScript idea! Currently I need to cast all my groups/datasets like this:
const attrs = group.attrs as unkown as MyAttrs;
Love the idea of typing files more strongly!
Just wanted to point out that the syntax below is a type assertion in disguise:
const file = h5wasm.File<MyHDF5File>(...)
This ESLint community rule argues against using this pattern. Perhaps it would be better to keep an explicit type cast?
Yes, please feel free to make a PR for typed Files/Groups.
Also, to answer your other points above:
- I think the test is fixed
- There is a DEVELOPER.md but it is not linked from README.md - and I need to add instructions for the typescript build in addition to the emscripten build.
- I did have esbuild working for a while but I switched to tsc because I wanted strict typescript compliance to be enforced as a side-effect of building instead of adding it as a separate testing step. If there is a better system for accomplishing this I am open to PRs on this as well.
const file = h5wasm.File<MyHDF5File>(...)
This ESLint community rule argues against using this pattern. Perhaps it would be better to keep an explicit type cast?
Thank you a lot for the feedback @axelboc! Like you said it is a type assertion, and I was not aware this is discouraged. Could you point out a syntax you would prefer? Based on the ESLint rule, something like this maybe:
const file: File<MyType> = h5wasm.File(...)
// or
const file = h5wasm.File(...) as File<MyFile>
Basically, h5wasm.File()
should return unknown
, which (if I'm not mistaken) would force the use of the as
syntax in strict mode. Though consumers would also be free to write a proper type guard/assertion:
function isMyFile(file: unknown): file is File<MyFile> {
return /* some condition that is sufficient to say that file is of type `File<MyFile>` */;
}
const file = h5wasm.File(..);
if (isMyFile(file)) {
// file has type // File<MyFile>
}