From dfeeafc084af3fe8b357a040ef89d0bcc97e4cb2 Mon Sep 17 00:00:00 2001 From: Oliver Beckstein Date: Thu, 9 Apr 2026 18:07:12 -0700 Subject: [PATCH 1/2] fix inconsistent error handling for Grid.load() - gOpenMol parser now captures its TypeError and raises ValueError if the header cannot be read; this makes it consistent with the other parsers - removed XFAILs from tests that check failure behavior for incorrectly specified file_format for Grid/Grid.load() - update CHANGELOG - update docs for Grid.load() --- CHANGELOG | 7 +++++-- gridData/core.py | 23 +++++++++++++++++++++++ gridData/gOpenMol.py | 17 +++++++++++------ gridData/tests/test_grid.py | 5 +---- 4 files changed, 40 insertions(+), 12 deletions(-) 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/core.py b/gridData/core.py index 035d769..94941a6 100644 --- a/gridData/core.py +++ b/gridData/core.py @@ -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..4bec50d 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 Exception as err: + raise ValueError(f"gOpenMol PLT file {filename} could not be read. " + "The 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..18859f8 100644 --- a/gridData/tests/test_grid.py +++ b/gridData/tests/test_grid.py @@ -179,12 +179,9 @@ def test_load_fileformat(self, data, pklfile, fileformat): h = Grid(pklfile, file_format="pkl") 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'): + with pytest.raises(ValueError): Grid(pklfile, file_format=fileformat) # just check that we can export without stupid failures; detailed From 51e2ca8887745f02fa598d6bdfa801bd7b50dcbc Mon Sep 17 00:00:00 2001 From: Oliver Beckstein Date: Thu, 9 Apr 2026 18:43:54 -0700 Subject: [PATCH 2/2] OpenDX parser: catch RecursionError on Windows and turn into ValueError --- gridData/OpenDX.py | 25 ++++++++++++++++++++++++- gridData/gOpenMol.py | 2 +- gridData/tests/test_grid.py | 4 ++-- 3 files changed, 27 insertions(+), 4 deletions(-) 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/gOpenMol.py b/gridData/gOpenMol.py index 4bec50d..78148a5 100644 --- a/gridData/gOpenMol.py +++ b/gridData/gOpenMol.py @@ -218,7 +218,7 @@ def read(self, filename): a = numpy.array(unpack(datafmt, plt.read(calcsize(datafmt)))) except Exception as err: raise ValueError(f"gOpenMol PLT file {filename} could not be read. " - "The error was\n" + "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!! diff --git a/gridData/tests/test_grid.py b/gridData/tests/test_grid.py index 18859f8..d097889 100644 --- a/gridData/tests/test_grid.py +++ b/gridData/tests/test_grid.py @@ -176,11 +176,11 @@ 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'] @pytest.mark.parametrize("fileformat", ("ccp4", "plt", "dx")) - def test_load_wrong_fileformat(self, data, pklfile, fileformat): + def test_load_wrong_fileformat_raises_ValueError(self, pklfile, fileformat): with pytest.raises(ValueError): Grid(pklfile, file_format=fileformat)