Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix error message for XDR readers #4231

Merged
merged 9 commits into from
Aug 11, 2023
13 changes: 6 additions & 7 deletions package/MDAnalysis/coordinates/XDR.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ def offsets_filename(filename, ending='npz'):

"""
head, tail = split(filename)
return join(head, '.{tail}_offsets.{ending}'.format(tail=tail,
ending=ending))
return join(head, f'.{tail}_offsets.{ending}')


def read_numpy_offsets(filename):
Expand All @@ -88,7 +87,7 @@ def read_numpy_offsets(filename):

# `ValueError` is encountered when the offset file is corrupted.
except (ValueError, IOError):
warnings.warn("Failed to load offsets file {}\n".format(filename))
warnings.warn(f"Failed to load offsets file {filename}\n")
return False

class XDRBaseReader(base.ReaderBase):
Expand Down Expand Up @@ -201,7 +200,7 @@ def _load_offsets(self):
except OSError as e:
if isinstance(e, PermissionError) or e.errno == errno.EROFS:
warnings.warn(f"Cannot write lock/offset file in same location as "
"{self.filename}. Using slow offset calculation.")
f"{self.filename}. Using slow offset calculation.")
self._read_offsets(store=True)
return
else:
Expand All @@ -220,10 +219,10 @@ def _load_offsets(self):
# refer to Issue #1893
data = read_numpy_offsets(fname)
if not data:
warnings.warn("Reading offsets from {} failed, "
warnings.warn(f"Reading offsets from {fname} failed, "
"reading offsets from trajectory instead.\n"
"Consider setting 'refresh_offsets=True' "
"when loading your Universe.".format(fname))
"when loading your Universe.")
self._read_offsets(store=True)
return

Expand Down Expand Up @@ -256,7 +255,7 @@ def _read_offsets(self, store=False):
offsets=offsets, size=size, ctime=ctime,
n_atoms=self._xdr.n_atoms)
except Exception as e:
warnings.warn("Couldn't save offsets because: {}".format(e))
warnings.warn(f"Couldn't save offsets because: {e}")

@property
def n_frames(self):
Expand Down
12 changes: 7 additions & 5 deletions testsuite/MDAnalysisTests/coordinates/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -443,13 +443,13 @@ def test_pickle_reader(self, reader):
assert_equal(len(reader), len(reader_p))
assert_equal(reader.ts, reader_p.ts,
"Timestep is changed after pickling")

def test_frame_collect_all_same(self, reader):
# check that the timestep resets so that the base reference is the same
# check that the timestep resets so that the base reference is the same
# for all timesteps in a collection with the exception of memoryreader
# and DCDReader
if isinstance(reader, mda.coordinates.memory.MemoryReader):
pytest.xfail("memoryreader allows independent coordinates")
pytest.xfail("memoryreader allows independent coordinates")
if isinstance(reader, mda.coordinates.DCD.DCDReader):
pytest.xfail("DCDReader allows independent coordinates."
"This behaviour is deprecated and will be changed"
Expand Down Expand Up @@ -493,9 +493,11 @@ def test_timeseries_asel_shape(self, reader, asel):
assert(timeseries.shape[0] == len(reader))
assert(timeseries.shape[1] == len(atoms))
assert(timeseries.shape[2] == 3)

def test_timeseries_empty_asel(self, reader):
atoms = mda.Universe(reader.filename).select_atoms(None)
with pytest.warns(UserWarning,
match="Empty string to select atoms, empty group returned."):
atoms = mda.Universe(reader.filename).select_atoms(None)
with pytest.raises(ValueError, match="Timeseries requires at least"):
reader.timeseries(atoms)

Expand Down
45 changes: 33 additions & 12 deletions testsuite/MDAnalysisTests/coordinates/test_xdr.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import pytest
from unittest.mock import patch

import re
import os
import shutil
import subprocess
Expand All @@ -47,6 +48,16 @@
from MDAnalysisTests.util import get_userid


@pytest.mark.parametrize("filename,kwargs,reference", [
orbeckst marked this conversation as resolved.
Show resolved Hide resolved
("foo.xtc", {}, ".foo.xtc_offsets.npz"),
("foo.xtc", {"ending": "npz"}, ".foo.xtc_offsets.npz"),
("bar.0001.trr", {"ending": "npzzzz"}, ".bar.0001.trr_offsets.npzzzz"),
])
def test_offsets_filename(filename, kwargs, reference):
fn = XDR.offsets_filename(filename, **kwargs)
assert fn == reference
orbeckst marked this conversation as resolved.
Show resolved Hide resolved


class _XDRReader_Sub(object):
@pytest.fixture()
def atoms(self):
Expand Down Expand Up @@ -212,7 +223,7 @@ def go_beyond_EOF():

with pytest.raises(StopIteration):
go_beyond_EOF()

def test_read_next_timestep_ts_no_positions(self, universe):
# primarily tests branching on ts.has_positions in _read_next_timestep
ts = universe.trajectory[0]
Expand Down Expand Up @@ -544,6 +555,7 @@ class _GromacsWriterIssue117(object):
def universe(self):
return mda.Universe(PRMncdf, NCDF)

@pytest.mark.filterwarnings("ignore: ATOMIC_NUMBER record not found")
def test_write_trajectory(self, universe, tmpdir):
"""Test writing Gromacs trajectories from AMBER NCDF (Issue 117)"""
outfile = str(tmpdir.join('xdr-writer-issue117' + self.ext))
Expand All @@ -559,10 +571,9 @@ def test_write_trajectory(self, universe, tmpdir):
written_ts._pos,
orig_ts._pos,
self.prec,
err_msg="coordinate mismatch "
"between original and written "
"trajectory at frame %d (orig) vs %d "
"(written)" % (orig_ts.frame, written_ts.frame))
err_msg=("coordinate mismatch between original and written "
f"trajectory at frame {orig_ts.frame:d} (orig) vs "
f"{orig_ts.frame:d} (written)"))


class TestXTCWriterIssue117(_GromacsWriterIssue117):
Expand Down Expand Up @@ -755,25 +766,34 @@ def test_nonexistent_offsets_file(self, traj):
outfile_offsets = XDR.offsets_filename(traj)
with patch.object(np, "load") as np_load_mock:
np_load_mock.side_effect = IOError
saved_offsets = XDR.read_numpy_offsets(outfile_offsets)
assert_equal(saved_offsets, False)
with pytest.warns(UserWarning, match=re.escape(
f"Failed to load offsets file {outfile_offsets}")):
saved_offsets = XDR.read_numpy_offsets(outfile_offsets)
assert saved_offsets == False

def test_nonexistent_offsets_file(self, traj):
def test_corrupted_offsets_file(self, traj):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, this shadowed the preceeding test; renaming increased our tests

# assert that a corrupted file returns False during read-in
# Issue #3230
outfile_offsets = XDR.offsets_filename(traj)
with patch.object(np, "load") as np_load_mock:
np_load_mock.side_effect = ValueError
saved_offsets = XDR.read_numpy_offsets(outfile_offsets)
assert_equal(saved_offsets, False)
with pytest.warns(UserWarning, match=re.escape(
f"Failed to load offsets file {outfile_offsets}")):
saved_offsets = XDR.read_numpy_offsets(outfile_offsets)
assert saved_offsets == False

def test_reload_offsets_if_offsets_readin_io_fails(self, trajectory):
# force the np.load call that is called in read_numpy_offsets
# during _load_offsets to give an IOError
# ensure that offsets are then read-in from the trajectory
with patch.object(np, "load") as np_load_mock:
np_load_mock.side_effect = IOError
trajectory._load_offsets()
with (pytest.warns(UserWarning,
match="Failed to load offsets file") and
pytest.warns(UserWarning,
match="reading offsets from trajectory instead")):
trajectory._load_offsets()

assert_almost_equal(
trajectory._xdr.offsets,
self.ref_offsets,
Expand Down Expand Up @@ -866,7 +886,8 @@ def test_unsupported_format(self, traj):
np.savez(fname, **saved_offsets)

# ok as long as this doesn't throw
reader = self._reader(traj)
with pytest.warns(UserWarning, match="Reload offsets from trajectory"):
reader = self._reader(traj)
reader[idx_frame]

@pytest.mark.skipif(get_userid() == 0, reason="cannot readonly as root")
Expand Down
Loading