Change nc_inq_var_deflate()/nc_inq_var_szip() to return NC_NOERR for non-HDF5 files, reporting compression off

[edwardhartnett]

In the 4.7.4 release, attempts to read the deflate level of a var in a classic file return an NC_ENOTNC4 error.

However, historically, the function returned a deflate level of 0 for that var.

Whether this make sense or not is less clear. If you think of deflate as invariably liked to filters, then it make sense to return failure. However, if you think of deflate as a compression algorithm that could be applied to multiple dispatch layers. then it makes sense to return 0 for those dispatch layers that don't support deflate.

[edwardhartnett]

Meanwhile, @DennisHeimbigner says:

I have been thinking about this. I think Ed's original instinct
here was right and that calling netcdf-4 specific functions
on netcdf-3 files should flag an error. In the reported case,
they should know what kind of file they have since they
are writing it.
=Dennis

[gsjaardema]

My problem with the current situation is that it breaks codes that used to work...

[DennisHeimbigner]

Good point, Ed. I had not thought of it in that way.

[czender]

FWIW 4.7.4 breaks three NCO ncks regression tests, too. Am investigating why. I suspect nc_inq_var_filter() prototype, which now probably needs to be made to depend on the netCDF library version in client programs (fun stuff), or because NCO may inquire about the deflate level of contiguous variables (which I think used to be allowed and would return 0?). I'm OK with changing all this because the new filter support is helpful, though the 4.7.4 changes are certainly not backward compatible as Greg mentions.

[DennisHeimbigner]

I guess we do a reversion.

[edhartnett]

OK, I have a PR I will put up tomorrow morning...

…

On Tue, Apr 7, 2020 at 6:32 PM Dennis Heimbigner ***@***.***> wrote: I guess we do a reversion. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1691 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABCSXXGM3BQZCMBFB3TPQ6LRLPAZJANCNFSM4MDMDQFA> .

[czender]

I incorrectly implied above that the prototype of nc_inq_var_filter() has changed. In fact the prototype has not changed (yay!). It is the behavior of the function that has changed in that it now returns errors in situations that it did not before. I am still happy to make the necessary changes to NCO, though if you think reverting this change is the right thing to do I won't complain since it could save me some work.

[DennisHeimbigner]

I think Ed's comment hits the right note that most of these
functions are actually format independent. Certainly our Zarr
format will have filter and chunking support.

[edwardhartnett]

I have added nc_inq_var_szip() to this change as well.

That is, both nc_inq_var_deflate() and nc_inq_var_szip() will no longer return NC_ENOTNC4 when called for a non-HDF5 file. Instead, they will return NC_NOERR, and report that compression is not in use.

As @DennisHeimbigner alludes, there may be other formats besides HDF5 which one day make better use of these functions. PR is up now, I'm just tinkering with tests and RELEASE_NOTES now...