netcdf-c icon indicating copy to clipboard operation
netcdf-c copied to clipboard

Silence most warnings in libhdf5

Open ZedThree opened this issue 1 year ago • 3 comments

The remaining warnings basically fall into two categories:

  • those from NC_ATT_INFO.len being int -- changing this to size_t would silence a lot of warnings in one go
  • those from HDF5 filters -- the filter type in HDF5 itself is H5Zfilter_t == int, whereas netcdf mostly uses unsigned int; and then the flags/parameters are exactly the other way round.

The filter types can't be completely fixed because they're in the netcdf public API, but we could fix the internals and just convert on the boundary. This is would change a bunch of internal API though, so I haven't done that here.

ZedThree avatar Mar 01 '24 17:03 ZedThree

The change to NC_ATT_INFO is probably ok since it is never visible on disk or to users. Changing the filter type is more problematic. It is visible to both the users our libnetcdf and libnetcdf's use of libhdf5, so the conversion would have occur in both situations. I think this also affects the filter wrappers in the plugins directory. Bottom line, this change may not be worth it at the moment.

DennisHeimbigner avatar Mar 01 '24 18:03 DennisHeimbigner

I agree about the NC_ATT_INFO len.

WRT to types for filter functions, the goal is to hide all HDF5 types behind standard C types. That is, I deliberately avoided using any HDF5 types in the netCDF API. I'm not sure if the filter code follows this rule.

I know of several advanced users using filters to great effect, including some ESA satellites which are using JPEG compression in netCDF data (IIRC in the recently-launched MTG-I1). So this is critical functionality that is already built into important operational systems.

edwardhartnett avatar Mar 02 '24 14:03 edwardhartnett

Ok, I changed NC_ATT_INFO.len -- it fixed 20 warnings and raised no new ones.

I might make a separate PR for the internal type of the filter ID so you can see the specific changes

ZedThree avatar Mar 04 '24 17:03 ZedThree