diff --git a/CHANGELOG b/CHANGELOG index b3fb690..1075061 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,9 +15,12 @@ The rules for this file: ------------------------------------------------------------------------------- ??/??/???? orbeckst - * 1.1.1 + * 1.2.0 - Fixes + Fixes + + * Ensure that when Grid() or Grid.load() load a datafile with the + wrong format a ValueError is raised consistently. (PR #165) 01/22/2026 IAlibay, ollyfutur, conradolandia, orbeckst, PlethoraChutney, diff --git a/gridData/OpenDX.py b/gridData/OpenDX.py index a081942..31e32f0 100644 --- a/gridData/OpenDX.py +++ b/gridData/OpenDX.py @@ -561,10 +561,33 @@ def read(self, stream): dx = OpenDX.field.read(dxfile) The classid is discarded and replaced with the one from the file. + + Parameters + ---------- + stream : str or stream + read from the file in OpenDX format or open file-like object + + Raises + ------ + ValueError + if the data could not be parsed correctly + + + .. versionchanged:: 1.2.0 + Ensure that ValueError is raised (instead of + :exc:`UnicodeDecodeError` or :exc:`RecursionError`) + """ DXfield = self p = DXParser(stream) - p.parse(DXfield) + try: + p.parse(DXfield) + except (UnicodeDecodeError, RecursionError) as err: + # parser got confused, likely not a valid file + # (RecursionError was only observed on Windows) + raise ValueError("DX file could not be read. " + "The original error was\n" + f" {err.__class__.__name__}: {err}") def add(self,component,DXobj): """add a component to the field""" diff --git a/gridData/core.py b/gridData/core.py index 035d769..0d981f4 100644 --- a/gridData/core.py +++ b/gridData/core.py @@ -536,7 +536,7 @@ def _load( raise NotImplementedError( "Non-rectangular grids are not supported.") elif len(delta) != grid.ndim: - raise TypeError("delta should be scalar or array-like of" + raise TypeError("delta should be scalar or array-like of " "len(grid.ndim)") # note that origin is CENTER so edges must be shifted by -0.5*delta self.edges = [origin[dim] + @@ -558,6 +558,29 @@ def load(self, filename, file_format=None, assume_volumetric=False): The :meth:`load` method calls the class's constructor method and completely resets all values, based on the loaded data. + + Parameters + ---------- + filename : str + Name of the file. + + file_format : str or None, optional + Set the file format (e.g., "DX" or "MRC"). If ``None`` then + try to guess the format. + + assume_volumetric : bool, optional + Optional keyword argument that is only taken into account by + the :class:`gridData.mrc.MRC` file parser. + + Raises + ------ + ValueError + The underlying file parser raises an :exc:`ValueError` if it fails + to parse the file. + + + .. versionchanged:: 1.2.0 + Ensure that underlying parsers consistently raise ValueError. """ filename = str(filename) if not os.path.exists(filename): diff --git a/gridData/gOpenMol.py b/gridData/gOpenMol.py index 3a2aa3c..ae98655 100644 --- a/gridData/gOpenMol.py +++ b/gridData/gOpenMol.py @@ -209,12 +209,17 @@ def read(self, filename): from struct import calcsize, unpack if not filename is None: self.filename = str(filename) - with open(self.filename, 'rb') as plt: - h = self.header = self._read_header(plt) - nentries = h['nx'] * h['ny'] * h['nz'] - # quick and dirty... slurp it all in one go - datafmt = h['bsaflag']+str(nentries)+self._data_bintype - a = numpy.array(unpack(datafmt, plt.read(calcsize(datafmt)))) + try: + with open(self.filename, 'rb') as plt: + h = self.header = self._read_header(plt) + nentries = h['nx'] * h['ny'] * h['nz'] + # quick and dirty... slurp it all in one go + datafmt = h['bsaflag'] + str(nentries) + self._data_bintype + a = numpy.array(unpack(datafmt, plt.read(calcsize(datafmt)))) + except TypeError as err: + raise ValueError(f"gOpenMol PLT file {filename} could not be read. " + "The original error was\n" + f" {err.__class__.__name__}: {err}") self.header['filename'] = self.filename self.array = a.reshape(h['nz'], h['ny'], h['nx']).transpose() # unpack plt in reverse!! self.delta = self._delta() diff --git a/gridData/tests/test_grid.py b/gridData/tests/test_grid.py index bc6c9c5..d6c9f9a 100644 --- a/gridData/tests/test_grid.py +++ b/gridData/tests/test_grid.py @@ -33,11 +33,15 @@ def test_init(self, data): assert_array_equal(g.delta, data['delta']) def test_init_wrong_origin(self, data): - with pytest.raises(TypeError): + with pytest.raises(TypeError, + match=(r"Dimension of origin is not the same as " + r"grid dimension\.")): Grid(data['griddata'], origin=np.ones(4), delta=data['delta']) def test_init_wrong_delta(self, data): - with pytest.raises(TypeError): + with pytest.raises(TypeError, + match=(r"delta should be scalar or array-like of " + r"len\(grid\.ndim\)")): Grid(data['griddata'], origin=data['origin'], delta=np.ones(4)) def test_empty_Grid(self): @@ -45,19 +49,24 @@ def test_empty_Grid(self): assert isinstance(g, Grid) def test_init_missing_delta_ValueError(self, data): - with pytest.raises(ValueError): + with pytest.raises(ValueError, + match="Wrong/missing data to set up Grid"): Grid(data['griddata'], origin=data['origin']) def test_init_missing_origin_ValueError(self, data): - with pytest.raises(ValueError): + with pytest.raises(ValueError, + match="Wrong/missing data to set up Grid"): Grid(data['griddata'], delta=data['delta']) def test_init_wrong_data_exception(self): - with pytest.raises(IOError): - Grid("__does_not_exist__") - - def test_load_wrong_fileformat_ValueError(self): - with pytest.raises(ValueError): + fn = "__does_not_exist__" + with pytest.raises(IOError, + match=r"\[Errno 2\] file not found:" + f" '{fn}'"): + Grid(fn) + + def test_load_unknown_fileformat_ValueError(self): + with pytest.raises(ValueError, + match="Wrong/missing data to set up Grid"): Grid(grid=True, file_format="xxx") def test_equality(self, data): @@ -113,16 +122,19 @@ def test_compatibility_type(self, data): def test_wrong_compatibile_type(self, data): g = Grid(data['griddata'], origin=data['origin'] + 1, delta=data['delta']) - with pytest.raises(TypeError): + with pytest.raises(TypeError, + match="The argument cannot be arithmetically combined"): data['grid'].check_compatible(g) arr = np.zeros(data['griddata'].shape[-1] + 1) # Not broadcastable - with pytest.raises(TypeError): + with pytest.raises(TypeError, + match="The argument cannot be arithmetically combined"): data['grid'].check_compatible(arr) def test_non_orthonormal_boxes(self, data): delta = np.eye(3) - with pytest.raises(NotImplementedError): + with pytest.raises(NotImplementedError, + match="Non-rectangular grids are not supported"): Grid(data['griddata'], origin=data['origin'], delta=delta) def test_centers(self, data): @@ -138,7 +150,7 @@ def test_centers(self, data): def test_resample_factor_failure(self, data): pytest.importorskip('scipy') - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="Factor must be positive"): g = data['grid'].resample_factor(0) def test_resample_factor(self, data): @@ -176,20 +188,19 @@ def test_init_pickle_pathobjects(self, data, tmpdir): @pytest.mark.parametrize("fileformat", ("pkl", "PKL", "pickle", "python")) def test_load_fileformat(self, data, pklfile, fileformat): - h = Grid(pklfile, file_format="pkl") + h = Grid(pklfile, file_format=fileformat) assert h == data['grid'] - # At the moment, reading the file with the wrong parser does not give - # good error messages. - @pytest.mark.xfail - @pytest.mark.parametrize("fileformat", ("ccp4", "plt", "dx")) - def test_load_wrong_fileformat(self, data, pklfile, fileformat): - with pytest.raises('ValueError'): + @pytest.mark.parametrize("fileformat", ("mrc", "plt", "dx")) + def test_load_wrong_fileformat_raises_ValueError(self, pklfile, fileformat): + # no error matching because ValueErrors can come from different + # parts of the underlying file parser and have different messages + with pytest.raises(ValueError): Grid(pklfile, file_format=fileformat) # just check that we can export without stupid failures; detailed # format checks in separate tests - @pytest.mark.parametrize("fileformat", ("dx", "pkl")) + @pytest.mark.parametrize("fileformat", ("dx", "mrc", "pkl")) def test_export(self, data, fileformat, tmpdir): g = data['grid'] fn = tmpdir.mkdir('grid_export').join("grid.{}".format(fileformat)) @@ -197,11 +208,12 @@ def test_export(self, data, fileformat, tmpdir): h = Grid(fn) # use format autodetection assert g == h - @pytest.mark.parametrize("fileformat", ("ccp4", "plt")) + @pytest.mark.parametrize("fileformat", ("plt",)) def test_export_not_supported(self, data, fileformat, tmpdir): g = data['grid'] - fn = tmpdir.mkdir('grid_export').join("grid.{}".format(fileformat)) - with pytest.raises(ValueError): + fn = tmpdir.mkdir('grid_export').join(f"grid.{fileformat}") + with pytest.raises(ValueError, + match=f"File format {fileformat.upper()} not available"): g.export(fn)