Repository: Unidata/netcdf-c (c)

Issue:
Change nc_inq_var_deflate()/nc_inq_var_szip() to return NC_NOERR for non-HDF5 files, reporting compression off
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.



Pull request:
now testing that nc_inq_var_deflate()/nc_inq_var_szip() work for all formats and returns settings consistent with no compression in use
Fixes #1691 

Patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index c19b8705a..e6c741156 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -8,6 +8,7 @@ This file contains a high-level description of this package's evolution. Release
 ## 4.8.0 - TBD
 
 * [Bug Fix][cmake] Correct an issue with parallel filter test logic in CMake-based builds.
+* [Bug Fix] Now allow nc_inq_var_deflate()/nc_inq_var_szip() to be called for all formats, not just HDF5. Non-HDF5 files return NC_NOERR and report no compression in use. This reverts behavior that was changed in the 4.7.4 release. See [https://github.com/Unidata/netcdf-c/issues/1691].
 * [Bug Fix] Compiling on a big-endian machine exposes some missing forward delcarations in dfilter.c.
 
 ## 4.7.4 - March 27, 2020
diff --git a/libdispatch/dvar.c b/libdispatch/dvar.c
index 91d93230d..143cbff8f 100644
--- a/libdispatch/dvar.c
+++ b/libdispatch/dvar.c
@@ -317,12 +317,16 @@ nc_def_var_fill(int ncid, int varid, int no_fill, const void *fill_value)
 }
 
 /**
-   Set the zlib compression settings for a netCDF-4/HDF5 variable.
+   Set the zlib compression and shuffle settings for a variable in an
+   netCDF/HDF5 file.
 
    This function must be called after nc_def_var and before nc_enddef
    or any functions which writes data to the file.
 
-   Deflation and shuffline require chunked data. If this function is
+   Deflation and shuffle are only available for HDF5 files. Attempting
+   to set them on non-HDF5 files will return ::NC_ENOTNC4.
+
+   Deflation and shuffle require chunked data. If this function is
    called on a variable with contiguous data, then the data is changed
    to chunked data, with default chunksizes. Use nc_def_var_chunking()
    to tune performance with user-defined chunksizes.
@@ -330,8 +334,9 @@ nc_def_var_fill(int ncid, int varid, int no_fill, const void *fill_value)
    If this function is called on a scalar variable, ::NC_EINVAL is
    returned. Only chunked variables may use filters.
 
-   If this function is called on a variable which already has szip
-   compression turned on, ::NC_EINVAL is returned.
+   Zlib compression cannot be used with szip compression. If this
+   function is called on a variable which already has szip compression
+   turned on, ::NC_EINVAL is returned.
 
    @note Parallel I/O reads work with compressed data. Parallel I/O
    writes work with compressed data in netcdf-c-4.7.4 and later
@@ -341,6 +346,28 @@ nc_def_var_fill(int ncid, int varid, int no_fill, const void *fill_value)
    variable in a file opened for parallel I/O will automatically
    switch the access for that variable to collective access.
 
+   @note The HDF5 manual has this to say about shuffle:
+   
+      The shuffle filter de-interlaces a block of data by reordering
+      the bytes. All the bytes from one consistent byte position of
+      each data element are placed together in one block; all bytes
+      from a second consistent byte position of each data element are
+      placed together a second block; etc. For example, given three
+      data elements of a 4-byte datatype stored as 012301230123,
+      shuffling will re-order data as 000111222333. This can be a
+      valuable step in an effective compression algorithm because the
+      bytes in each byte position are often closely related to each
+      other and putting them together can increase the compression
+      ratio.
+
+      As implied above, the primary value of the shuffle filter lies
+      in its coordinated use with a compression filter; it does not
+      provide data compression when used alone. When the shuffle
+      filter is applied to a dataset immediately prior to the use of a
+      compression filter, the compression ratio achieved is often
+      superior to that achieved by the use of a compression filter
+      without the shuffle filter.
+
    @param ncid NetCDF or group ID, from a previous call to nc_open(),
    nc_create(), nc_def_grp(), or associated inquiry functions such as
    nc_inq_ncid().
diff --git a/libdispatch/dvarinq.c b/libdispatch/dvarinq.c
index caad7e7b8..5f720450c 100644
--- a/libdispatch/dvarinq.c
+++ b/libdispatch/dvarinq.c
@@ -255,8 +255,18 @@ nc_inq_varnatts(int ncid, int varid, int *nattsp)
 		     nattsp);
 }
 
-/** \ingroup variables
-Learn the storage and deflate settings for a variable.
+/** 
+\ingroup variables 
+
+Learn the shuffle and deflate settings for a variable.
+
+Deflation is compression with the zlib library. Shuffle re-orders the
+data bytes to provide better compression (see nc_def_var_deflate()).
+
+Deflation is only available for HDF5 files. For classic and other
+files, this function will return setting that indicate that deflation
+is not in use, and that the shuffle filter is not in use. That is:
+shuffle off, deflate off, and a deflate level of 0.
 
 \param ncid NetCDF or group ID, from a previous call to nc_open(),
 nc_create(), nc_def_grp(), or associated inquiry functions such as
@@ -277,7 +287,6 @@ use, and deflate_levelp is provided, it will get a zero. (This
 behavior is expected by the Fortran APIs). \ref ignored_if_null.
 
 \returns ::NC_NOERR No error.
-\returns ::NC_ENOTNC4 Not a netCDF-4 file.
 \returns ::NC_EBADID Bad ncid.
 \returns ::NC_ENOTVAR Invalid variable ID.
 \author Ed Hartnett, Dennis Heimbigner
@@ -289,8 +298,9 @@ nc_inq_var_deflate(int ncid, int varid, int *shufflep, int *deflatep, int *defla
    size_t nparams;
    unsigned int params[4];
    int deflating = 0;
-
-   int stat = NC_check_id(ncid,&ncp);
+   int stat;
+   
+   stat = NC_check_id(ncid,&ncp);
    if(stat != NC_NOERR) return stat;
    TRACE(nc_inq_var_deflate);
 
@@ -299,6 +309,18 @@ nc_inq_var_deflate(int ncid, int varid, int *shufflep, int *deflatep, int *defla
    switch (stat) {
    case NC_ENOFILTER: deflating = 0; stat = NC_NOERR; break;
    case NC_NOERR: deflating = 1; break;
+   case NC_ENOTNC4:
+       /* As a special case, to support behavior already coded into user
+	* applications, handle classic format files by reporting no
+	* deflation. */
+       if (shufflep)
+	   *shufflep = 0;
+       if (deflatep)
+	   *deflatep = 0;
+       if (deflate_levelp)
+	   *deflate_levelp = 0;
+       return NC_NOERR;
+       break;
    default: return stat;
    }
    if(deflatep) *deflatep = deflating;
@@ -637,16 +659,20 @@ Learn the szip settings of a variable.
 
 This function returns the szip settings for a variable. To turn on
 szip compression, use nc_def_var_szip(). Szip compression is only
-available if HDF5 was built with szip support. The nc_def_var_filter
-function may also be used to set szip compression.
+available for netCDF/HDF5 files, and only if HDF5 was built with szip
+support. 
 
-If a variable is not using szip, then a zero will be passed back
-for both options_maskp and pixels_per_blockp.
+If a variable is not using szip, or if this function is called on a
+file that is not a HDF5 file, then a zero will be passed back for both
+options_maskp and pixels_per_blockp.
 
 For more information on HDF5 and szip see
 https://support.hdfgroup.org/HDF5/doc/RM/RM_H5P.html#Property-SetSzip
 and https://support.hdfgroup.org/doc_resource/SZIP/index.html.
 
+The nc_def_var_filter function may also be used to set szip
+compression.
+
 \param ncid NetCDF or group ID, from a previous call to nc_open(),
 nc_create(), nc_def_grp(), or associated inquiry functions such as
 nc_inq_ncid().
@@ -667,12 +693,10 @@ szip is not in use for this variable. \ref ignored_if_null.
 
 \returns ::NC_NOERR No error.
 \returns ::NC_EBADID Bad ncid.
-\returns ::NC_ENOTNC4 Not a netCDF-4 file.
 \returns ::NC_ENOTVAR Invalid variable ID.
 \returns ::NC_EFILTER Filter error.
 
 \author Ed Hartnett, Dennis Heimbigner
-
 */
 int
 nc_inq_var_szip(int ncid, int varid, int *options_maskp, int *pixels_per_blockp)
@@ -693,7 +717,9 @@ nc_inq_var_szip(int ncid, int varid, int *options_maskp, int *pixels_per_blockp)
 	    return NC_EFILTER; /* bad # params */
 	break;
    case NC_ENOFILTER:
-       /* If the szip filter is not in use, return 0 for both  parameters. */
+   case NC_ENOTNC4:
+       /* If the szip filter is not in use, or if this is not a HDF5
+	* file, return 0 for both parameters. */
        params[0] = 0;
        params[1] = 0;
        stat = NC_NOERR;


Test patch:
diff --git a/nc_test/tst_formats.c b/nc_test/tst_formats.c
index 251026d27..2bf63b4be 100644
--- a/nc_test/tst_formats.c
+++ b/nc_test/tst_formats.c
@@ -171,6 +171,8 @@ main(int argc, char **argv)
                int data = TEST_VAL_42;
                int data_in;
                int fill_value = TEST_VAL_42 * 2;
+	       int shuffle_in, deflate_in, deflate_level_in;
+	       int options_mask_in, pixels_per_block_in;
 
                /* Try to set fill mode after data have been written. */
                sprintf(file_name, "%s_%d_%d_%d_elatefill.nc", FILE_NAME_BASE, format[f], d, a);
@@ -178,6 +180,18 @@ main(int argc, char **argv)
                if (nc_create(file_name, 0, &ncid)) ERR;
                if (nc_def_dim(ncid, DIM_NAME, DIM_LEN, &dimid)) ERR;
                if (nc_def_var(ncid, VAR_NAME, NC_INT, NDIM1, &dimid, &varid)) ERR;
+
+	       /* There is no deflate on this var, and that is true in
+		* all formats. */
+	       if (nc_inq_var_deflate(ncid, varid, &shuffle_in, &deflate_in,
+				      &deflate_level_in)) ERR;
+	       if (shuffle_in || deflate_in || deflate_level_in) ERR;
+	       
+	       /* There is no szip on this var, and that is true in
+		* all formats. */
+	       if (nc_inq_var_szip(ncid, varid, &options_mask_in, &pixels_per_block_in)) ERR;
+	       if (options_mask_in || pixels_per_block_in) ERR;
+	       
                if (nc_enddef(ncid)) ERR;
                /* For netCDF-4, we don't actually have to write data to
                 * prevent future setting of the fill value. Once the user
@@ -208,6 +222,18 @@ main(int argc, char **argv)
                   if (ret != (a ? NC_ELATEDEF: NC_ELATEFILL)) ERR;
                }
                if (nc_enddef(ncid)) ERR;
+
+	       /* There is (still!) no deflate on this var, and that
+		* is true in all formats. */
+	       if (nc_inq_var_deflate(ncid, varid, &shuffle_in, &deflate_in,
+				      &deflate_level_in)) ERR;
+	       if (shuffle_in || deflate_in || deflate_level_in) ERR;
+	       
+	       /* There is (still!) no szip on this var, and that is
+		* true in all formats. */
+	       if (nc_inq_var_szip(ncid, varid, &options_mask_in, &pixels_per_block_in)) ERR;
+	       if (options_mask_in || pixels_per_block_in) ERR;
+	       
                if (nc_close(ncid)) ERR;
 
                /* Open the file and check data. */