Skip to content

Commit accce2b

Browse files
authored
fix inconsistent error handling for Grid.load() (#165)
* 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 docs for Grid.load() * OpenDX parser: catch RecursionError on Windows and turn into ValueError * fix exception message typo * improve exception testing by partially matching messages (not feasible for the ValueError raised when loading the wrong file format because these errors are not uniform * update CHANGELOG - add match testing for many Grid exceptions - specifically catch TypeError for gOpenMol instead of all Exceptions - fixed minor space typo in exception message
1 parent f45d52b commit accce2b

5 files changed

Lines changed: 100 additions & 34 deletions

File tree

CHANGELOG

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@ The rules for this file:
1515
-------------------------------------------------------------------------------
1616
??/??/???? orbeckst
1717

18-
* 1.1.1
18+
* 1.2.0
1919

20-
Fixes
20+
Fixes
21+
22+
* Ensure that when Grid() or Grid.load() load a datafile with the
23+
wrong format a ValueError is raised consistently. (PR #165)
2124

2225

2326
01/22/2026 IAlibay, ollyfutur, conradolandia, orbeckst, PlethoraChutney,

gridData/OpenDX.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,10 +561,33 @@ def read(self, stream):
561561
dx = OpenDX.field.read(dxfile)
562562
563563
The classid is discarded and replaced with the one from the file.
564+
565+
Parameters
566+
----------
567+
stream : str or stream
568+
read from the file in OpenDX format or open file-like object
569+
570+
Raises
571+
------
572+
ValueError
573+
if the data could not be parsed correctly
574+
575+
576+
.. versionchanged:: 1.2.0
577+
Ensure that ValueError is raised (instead of
578+
:exc:`UnicodeDecodeError` or :exc:`RecursionError`)
579+
564580
"""
565581
DXfield = self
566582
p = DXParser(stream)
567-
p.parse(DXfield)
583+
try:
584+
p.parse(DXfield)
585+
except (UnicodeDecodeError, RecursionError) as err:
586+
# parser got confused, likely not a valid file
587+
# (RecursionError was only observed on Windows)
588+
raise ValueError("DX file could not be read. "
589+
"The original error was\n"
590+
f" {err.__class__.__name__}: {err}")
568591

569592
def add(self,component,DXobj):
570593
"""add a component to the field"""

gridData/core.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ def _load(
536536
raise NotImplementedError(
537537
"Non-rectangular grids are not supported.")
538538
elif len(delta) != grid.ndim:
539-
raise TypeError("delta should be scalar or array-like of"
539+
raise TypeError("delta should be scalar or array-like of "
540540
"len(grid.ndim)")
541541
# note that origin is CENTER so edges must be shifted by -0.5*delta
542542
self.edges = [origin[dim] +
@@ -558,6 +558,29 @@ def load(self, filename, file_format=None, assume_volumetric=False):
558558
559559
The :meth:`load` method calls the class's constructor method and
560560
completely resets all values, based on the loaded data.
561+
562+
Parameters
563+
----------
564+
filename : str
565+
Name of the file.
566+
567+
file_format : str or None, optional
568+
Set the file format (e.g., "DX" or "MRC"). If ``None`` then
569+
try to guess the format.
570+
571+
assume_volumetric : bool, optional
572+
Optional keyword argument that is only taken into account by
573+
the :class:`gridData.mrc.MRC` file parser.
574+
575+
Raises
576+
------
577+
ValueError
578+
The underlying file parser raises an :exc:`ValueError` if it fails
579+
to parse the file.
580+
581+
582+
.. versionchanged:: 1.2.0
583+
Ensure that underlying parsers consistently raise ValueError.
561584
"""
562585
filename = str(filename)
563586
if not os.path.exists(filename):

gridData/gOpenMol.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -209,12 +209,17 @@ def read(self, filename):
209209
from struct import calcsize, unpack
210210
if not filename is None:
211211
self.filename = str(filename)
212-
with open(self.filename, 'rb') as plt:
213-
h = self.header = self._read_header(plt)
214-
nentries = h['nx'] * h['ny'] * h['nz']
215-
# quick and dirty... slurp it all in one go
216-
datafmt = h['bsaflag']+str(nentries)+self._data_bintype
217-
a = numpy.array(unpack(datafmt, plt.read(calcsize(datafmt))))
212+
try:
213+
with open(self.filename, 'rb') as plt:
214+
h = self.header = self._read_header(plt)
215+
nentries = h['nx'] * h['ny'] * h['nz']
216+
# quick and dirty... slurp it all in one go
217+
datafmt = h['bsaflag'] + str(nentries) + self._data_bintype
218+
a = numpy.array(unpack(datafmt, plt.read(calcsize(datafmt))))
219+
except TypeError as err:
220+
raise ValueError(f"gOpenMol PLT file {filename} could not be read. "
221+
"The original error was\n"
222+
f" {err.__class__.__name__}: {err}")
218223
self.header['filename'] = self.filename
219224
self.array = a.reshape(h['nz'], h['ny'], h['nx']).transpose() # unpack plt in reverse!!
220225
self.delta = self._delta()

gridData/tests/test_grid.py

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,31 +33,40 @@ def test_init(self, data):
3333
assert_array_equal(g.delta, data['delta'])
3434

3535
def test_init_wrong_origin(self, data):
36-
with pytest.raises(TypeError):
36+
with pytest.raises(TypeError,
37+
match=(r"Dimension of origin is not the same as "
38+
r"grid dimension\.")):
3739
Grid(data['griddata'], origin=np.ones(4), delta=data['delta'])
3840

3941
def test_init_wrong_delta(self, data):
40-
with pytest.raises(TypeError):
42+
with pytest.raises(TypeError,
43+
match=(r"delta should be scalar or array-like of "
44+
r"len\(grid\.ndim\)")):
4145
Grid(data['griddata'], origin=data['origin'], delta=np.ones(4))
4246

4347
def test_empty_Grid(self):
4448
g = Grid()
4549
assert isinstance(g, Grid)
4650

4751
def test_init_missing_delta_ValueError(self, data):
48-
with pytest.raises(ValueError):
52+
with pytest.raises(ValueError,
53+
match="Wrong/missing data to set up Grid"):
4954
Grid(data['griddata'], origin=data['origin'])
5055

5156
def test_init_missing_origin_ValueError(self, data):
52-
with pytest.raises(ValueError):
57+
with pytest.raises(ValueError,
58+
match="Wrong/missing data to set up Grid"):
5359
Grid(data['griddata'], delta=data['delta'])
5460

5561
def test_init_wrong_data_exception(self):
56-
with pytest.raises(IOError):
57-
Grid("__does_not_exist__")
58-
59-
def test_load_wrong_fileformat_ValueError(self):
60-
with pytest.raises(ValueError):
62+
fn = "__does_not_exist__"
63+
with pytest.raises(IOError,
64+
match=r"\[Errno 2\] file not found:" + f" '{fn}'"):
65+
Grid(fn)
66+
67+
def test_load_unknown_fileformat_ValueError(self):
68+
with pytest.raises(ValueError,
69+
match="Wrong/missing data to set up Grid"):
6170
Grid(grid=True, file_format="xxx")
6271

6372
def test_equality(self, data):
@@ -113,16 +122,19 @@ def test_compatibility_type(self, data):
113122

114123
def test_wrong_compatibile_type(self, data):
115124
g = Grid(data['griddata'], origin=data['origin'] + 1, delta=data['delta'])
116-
with pytest.raises(TypeError):
125+
with pytest.raises(TypeError,
126+
match="The argument cannot be arithmetically combined"):
117127
data['grid'].check_compatible(g)
118128

119129
arr = np.zeros(data['griddata'].shape[-1] + 1) # Not broadcastable
120-
with pytest.raises(TypeError):
130+
with pytest.raises(TypeError,
131+
match="The argument cannot be arithmetically combined"):
121132
data['grid'].check_compatible(arr)
122133

123134
def test_non_orthonormal_boxes(self, data):
124135
delta = np.eye(3)
125-
with pytest.raises(NotImplementedError):
136+
with pytest.raises(NotImplementedError,
137+
match="Non-rectangular grids are not supported"):
126138
Grid(data['griddata'], origin=data['origin'], delta=delta)
127139

128140
def test_centers(self, data):
@@ -138,7 +150,7 @@ def test_centers(self, data):
138150
def test_resample_factor_failure(self, data):
139151
pytest.importorskip('scipy')
140152

141-
with pytest.raises(ValueError):
153+
with pytest.raises(ValueError, match="Factor must be positive"):
142154
g = data['grid'].resample_factor(0)
143155

144156
def test_resample_factor(self, data):
@@ -176,32 +188,32 @@ def test_init_pickle_pathobjects(self, data, tmpdir):
176188

177189
@pytest.mark.parametrize("fileformat", ("pkl", "PKL", "pickle", "python"))
178190
def test_load_fileformat(self, data, pklfile, fileformat):
179-
h = Grid(pklfile, file_format="pkl")
191+
h = Grid(pklfile, file_format=fileformat)
180192
assert h == data['grid']
181193

182-
# At the moment, reading the file with the wrong parser does not give
183-
# good error messages.
184-
@pytest.mark.xfail
185-
@pytest.mark.parametrize("fileformat", ("ccp4", "plt", "dx"))
186-
def test_load_wrong_fileformat(self, data, pklfile, fileformat):
187-
with pytest.raises('ValueError'):
194+
@pytest.mark.parametrize("fileformat", ("mrc", "plt", "dx"))
195+
def test_load_wrong_fileformat_raises_ValueError(self, pklfile, fileformat):
196+
# no error matching because ValueErrors can come from different
197+
# parts of the underlying file parser and have different messages
198+
with pytest.raises(ValueError):
188199
Grid(pklfile, file_format=fileformat)
189200

190201
# just check that we can export without stupid failures; detailed
191202
# format checks in separate tests
192-
@pytest.mark.parametrize("fileformat", ("dx", "pkl"))
203+
@pytest.mark.parametrize("fileformat", ("dx", "mrc", "pkl"))
193204
def test_export(self, data, fileformat, tmpdir):
194205
g = data['grid']
195206
fn = tmpdir.mkdir('grid_export').join("grid.{}".format(fileformat))
196207
g.export(fn) # check that path objects work
197208
h = Grid(fn) # use format autodetection
198209
assert g == h
199210

200-
@pytest.mark.parametrize("fileformat", ("ccp4", "plt"))
211+
@pytest.mark.parametrize("fileformat", ("plt",))
201212
def test_export_not_supported(self, data, fileformat, tmpdir):
202213
g = data['grid']
203-
fn = tmpdir.mkdir('grid_export').join("grid.{}".format(fileformat))
204-
with pytest.raises(ValueError):
214+
fn = tmpdir.mkdir('grid_export').join(f"grid.{fileformat}")
215+
with pytest.raises(ValueError,
216+
match=f"File format {fileformat.upper()} not available"):
205217
g.export(fn)
206218

207219

0 commit comments

Comments
 (0)