add VLType and CompoundType, commit complex compound type to file

[kmuehlbauer]

• Closes Issue #xxxx
• Tests added
• Changes are documented in CHANGELOG.rst
• fix char array issue for compound type

see #228 for transient vs committed types

[kmuehlbauer]

@ZedThree FYI, this is the first serve of adding the missing user types (committed types) to h5netcdf as well as making it compatible with nc-complex.

[ZedThree]

LGTM! I think your xarray tests are failing because, somewhat ironically, they actually work and you expect them to fail! 😄

Just to note that nc-complex supports a wider variety of conventions for complex numbers, but I think it's absolutely fine for h5netcdf to only support the h5py/transient compound type and the explicit _PFNC_*_COMPLEX named type. h5netcdf will be able to read them all perfectly ok, just not auto-detect them as being complex numbers.

[kmuehlbauer]

Thanks for taking a look here, much appreciated.

LGTM! I think your xarray tests are failing because, somewhat ironically, they actually work and you expect them to fail! 😄

Yes, they are actually from xarray's own test-suite, which expect h5netcdf to error here 😀

Just to note that nc-complex supports a wider variety of conventions for complex numbers, but I think it's absolutely fine for h5netcdf to only support the h5py/transient compound type and the explicit _PFNC_*_COMPLEX named type. h5netcdf will be able to read them all perfectly ok, just not auto-detect them as being complex numbers.

Sure, we can think about aligning h5netcdf's legacyapi to what netcdf4-python supports.

[ZedThree]

Sure, we can think about aligning h5netcdf's legacyapi to what netcdf4-python supports.

This would be neat, but much less important than your work here to ensure that currently released (and older) versions of netcdf will be able to read files created by h5netcdf. This is a great improvement for compatibility, so thank you!

[kmuehlbauer]

@shoyer, I'd like to merge this in the coming days. If you have time it would be great if you could take a look here. Thanks!

With the work of @ZedThree over in https://github.com/PlasmaFAIR/nc-complex and this PR here, we can enable complex numbers for xarray netcdf4/h5netcdf backends.

[kmuehlbauer]

I could not get around to check this in more detail in this gist: https://gist.github.com/kmuehlbauer/1f6f8a61ad5b0a7693ae08154060b790

It looks like we could get away by just committing the complex compound type to file as nectdf-c/netcdf4-python does instead of relying on the transient hdf5 type. This could also be considered as bug.

That means we could significantly simplify this PR. We would not have to use the auto_complex keyword, since we would write perfectly sane NetCDF4 files. All these files are readable with former netcdf-c/netcdf4-python versions as well as h5netcdf versions. This would be fully backwards compatible.

[kmuehlbauer]

@shoyer Thanks for the immediate review. I've fixed the mentioned issues and applied your change for exception handling.

Sure, xarray's h5netcdf tests (https://github.com/h5netcdf/h5netcdf/actions/runs/7597039010/job/20691426586?pr=227) are breaking, because of the complex compatibility tests over there. This will immediately lead to breaking nightly tests over at xarray, if this PR is merged. I'm ready to take action over at xarray.

[kmuehlbauer]

Unfortunately there are still some inconsistencies in the correct retrieval of the committed type. I'll push more changes later.

[kmuehlbauer]

I've added some comments for better understanding.

[kmuehlbauer]

This seems to work now as intended. I'll split out the changes regarding EnumType and shared user type into a separate PR's. This will finally wrap up VLType and CompoundType.

[kmuehlbauer]

@shoyer @ZedThree

After #229 and #230 (some refactoring to keep good review experience), I've updated this PR with the undisputed changes to get VL and Compound types working for h5netcdf. Plays well together with nc-complex AFAICT.

I'll do some more clean-up the next days, but you might already have a look here.

[kmuehlbauer]

I think this is ready for final review. I really like to get this in sometime next week, and get a release out.

[kmuehlbauer]

So, this is unfortunately not finished yet. There are issues with compound type containing strings. This is done by array of chars under the hood, which needs some special casing to be transparent in h5netcdf. See: Unidata/netcdf4-python#773

I'll set this PR to draft until this is figured out.

[kmuehlbauer]

@ZedThree @shoyer Finally I got around to finish this up. Please have a look if you are around.

I'm having another self-review early next week and will merge after that.

[ZedThree]

LGTM! Thanks @kmuehlbauer !