From 5fa2e57e162903914966cd43c1ce4ba634d4c070 Mon Sep 17 00:00:00 2001 From: ljwoods2 <145226270+ljwoods2@users.noreply.github.com> Date: Mon, 12 Feb 2024 06:19:22 -0700 Subject: [PATCH] ChainedReader __repr__ fix for MemoryReader sub-readers (#4407) * Reset branch and added pytest * PEP8 and CHANGELOG * Assert statement fix * Full iteration and repr() change * typo * typo * Update package/CHANGELOG Co-authored-by: Rocco Meli * Added direct test to u.trajectory.filenames * Removing print --------- Co-authored-by: Rocco Meli --- package/CHANGELOG | 5 +++- package/MDAnalysis/coordinates/chain.py | 24 ++++++++++++++----- .../coordinates/test_chainreader.py | 19 +++++++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index d4ba2ca05e7..c825d7094df 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -14,11 +14,14 @@ The rules for this file: ------------------------------------------------------------------------------- -??/??/?? IAlibay, HeetVekariya, marinegor, lilyminium, RMeli +??/??/?? IAlibay, HeetVekariya, marinegor, lilyminium, RMeli, + ljwoods2 * 2.8.0 Fixes + * Fix ChainReader `__repr__()` method when sub-reader is MemoryReader + (Issue #3349, PR #4407) * Fix bug in PCA preventing use of `frames=...` syntax (PR #4423) * Fix `analysis/diffusionmap.py` iteration through trajectory to iteration over `self._sliced_trajectory`, hence supporting diff --git a/package/MDAnalysis/coordinates/chain.py b/package/MDAnalysis/coordinates/chain.py index bdfdeff3523..0c09a596d95 100644 --- a/package/MDAnalysis/coordinates/chain.py +++ b/package/MDAnalysis/coordinates/chain.py @@ -269,8 +269,17 @@ def __init__(self, filenames, skip=1, dt=None, continuous=False, kwargs['dt'] = dt self.readers = [core.reader(filename, convert_units=convert_units, **kwargs) for filename in filenames] - self.filenames = np.array([fn[0] if isinstance(fn, tuple) else fn - for fn in filenames]) + # Iterate through all filenames, appending NoneType None for ndarrays + self.filenames = [] + for fn in filenames: + if isinstance(fn, np.ndarray): + self.filenames.append(None) + elif isinstance(fn, tuple): + self.filenames.append(fn[0]) + else: + self.filenames.append(fn) + self.filenames = np.array(self.filenames) + # pointer to "active" trajectory index into self.readers self.__active_reader_index = 0 @@ -598,11 +607,14 @@ def close(self): def __repr__(self): if len(self.filenames) > 3: - fnames = "{fname} and {nfanmes} more".format( - fname=os.path.basename(self.filenames[0]), - nfanmes=len(self.filenames) - 1) + fname = (os.path.basename(self.filenames[0]) + if self.filenames[0] else "numpy.ndarray") + fnames = "{fname} and {nfnames} more".format( + fname=fname, + nfnames=len(self.filenames) - 1) else: - fnames = ", ".join([os.path.basename(fn) for fn in self.filenames]) + fnames = ", ".join([os.path.basename(fn) if fn else "numpy.ndarray" + for fn in self.filenames]) return ("<{clsname} containing {fname} with {nframes} frames of {natoms} atoms>" "".format( clsname=self.__class__.__name__, diff --git a/testsuite/MDAnalysisTests/coordinates/test_chainreader.py b/testsuite/MDAnalysisTests/coordinates/test_chainreader.py index ba0687ffd76..2049b687279 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_chainreader.py +++ b/testsuite/MDAnalysisTests/coordinates/test_chainreader.py @@ -455,3 +455,22 @@ def wrapped(ts): # in the issue report values of ~25 were seen before the fix # with the fix all values should be <1e-6 assert np.abs(com).max() < 1e-6 + + +def test_issue_3349(): + # test for repr of a chainreader when the subreaders are memoryreaders + cdx = np.random.random(3341 * 3 * 10) + cdx = cdx.reshape(-1, 3341, 3).astype(np.float32) * 10 + u = mda.Universe(PSF, (cdx, DCD)) + u2 = mda.Universe(PSF, (cdx, cdx, cdx, DCD)) + u_expected_filenames = np.array([None, str(DCD)]) + u2_expected_filenames = np.array([None, None, None, str(DCD)]) + + assert_equal("", + u.trajectory.__repr__()) + assert_equal(u_expected_filenames, u.trajectory.filenames) + assert_equal("", + u2.trajectory.__repr__()) + assert_equal(u2_expected_filenames, u2.trajectory.filenames)