diff --git a/src/hdmf/backends/hdf5/h5_utils.py b/src/hdmf/backends/hdf5/h5_utils.py index a893746ef..79ed45d8c 100644 --- a/src/hdmf/backends/hdf5/h5_utils.py +++ b/src/hdmf/backends/hdf5/h5_utils.py @@ -310,20 +310,30 @@ def _check_compression_options(self): if 'compression_opts' in self.__iosettings: if self.__iosettings['compression'] == 'gzip': if self.__iosettings['compression_opts'] not in range(10): - raise ValueError("GZIP compression_opts setting must be an integer from 0-9, " + + raise ValueError("GZIP compression_opts setting must be an integer from 0-9, " "not " + str(self.__iosettings['compression_opts'])) elif self.__iosettings['compression'] == 'lzf': if self.__iosettings['compression_opts'] is not None: raise ValueError("LZF compression filter accepts no compression_opts") elif self.__iosettings['compression'] == 'szip': - if not isinstance(self.__iosettings['compression_opts'], tuple) or \ - len(self.__iosettings['compression_opts']) != 2: - raise ValueError("SZIP compression filter compression_opts" + - " must be a 2-tuple ('ec'|'nn', even integer 0-32") + szip_opts_error = False + # Check that we have a tuple + szip_opts_error |= not isinstance(self.__iosettings['compression_opts'], tuple) + # Check that we have a tuple of the right length and correct settings + if not szip_opts_error: + try: + szmethod, szpix = self.__iosettings['compression_opts'] + szip_opts_error |= (szmethod not in ('ec', 'nn')) + szip_opts_error |= (not (0 < szpix <= 32 and szpix % 2 == 0)) + except ValueError: # ValueError is raised if tuple does not have the right length to unpack + szip_opts_error = True + if szip_opts_error: + raise ValueError("SZIP compression filter compression_opts" + " must be a 2-tuple ('ec'|'nn', even integer 0-32).") # Warn if compressor other than gzip is being used if self.__iosettings['compression'] != 'gzip': - warnings.warn(str(self.__iosettings['compression']) + " compression may not be available" + - "on all installations of HDF5. Use of gzip is recommended to ensure portability of" + + warnings.warn(str(self.__iosettings['compression']) + " compression may not be available" + "on all installations of HDF5. Use of gzip is recommended to ensure portability of" "the generated HDF5 files.") @staticmethod diff --git a/tests/unit/test_io_hdf5_h5tools.py b/tests/unit/test_io_hdf5_h5tools.py index 37f8967fb..2202ea46e 100644 --- a/tests/unit/test_io_hdf5_h5tools.py +++ b/tests/unit/test_io_hdf5_h5tools.py @@ -441,7 +441,7 @@ def test_warning_on_non_gzip_compression(self): compression='gzip') self.assertEqual(len(w), 0) self.assertEqual(dset.io_settings['compression'], 'gzip') - # Make sure no warning is issued when using szip (if installed) + # Make sure a warning is issued when using szip (even if installed) if "szip" in h5py_filters.encode: with warnings.catch_warnings(record=True) as w: dset = H5DataIO(np.arange(30), @@ -452,7 +452,7 @@ def test_warning_on_non_gzip_compression(self): else: with self.assertRaises(ValueError): H5DataIO(np.arange(30), compression='szip', compression_opts=('ec', 16)) - # Make sure no warning is issued when using lzf + # Make sure a warning is issued when using lzf compression with warnings.catch_warnings(record=True) as w: dset = H5DataIO(np.arange(30), compression='lzf') @@ -494,6 +494,12 @@ def test_value_error_on_incompatible_compression_opts(self): # Make sure we warn if szip with gzip compression option is used with self.assertRaises(ValueError): H5DataIO(np.arange(30), compression='szip', compression_opts=4) + # Make sure szip raises a ValueError if bad options are used (odd compression option) + with self.assertRaises(ValueError): + H5DataIO(np.arange(30), compression='szip', compression_opts=('ec', 3)) + # Make sure szip raises a ValueError if bad options are used (bad methos) + with self.assertRaises(ValueError): + H5DataIO(np.arange(30), compression='szip', compression_opts=('bad_method', 16)) def test_warning_on_linking_of_regular_array(self): with warnings.catch_warnings(record=True) as w: