From a474dd3d15dacfe2d25085fd5756be7e5874b8e0 Mon Sep 17 00:00:00 2001 From: Hugo MacDermott-Opeskin Date: Sun, 3 Dec 2023 20:20:26 +1100 Subject: [PATCH] Add warning for future change of argument name for `timeseries` from `asel` -> `atomgroup` (#4343) --- package/CHANGELOG | 2 ++ package/MDAnalysis/coordinates/DCD.py | 21 ++++++++++++--- package/MDAnalysis/coordinates/base.py | 21 ++++++++++++--- package/MDAnalysis/coordinates/memory.py | 26 +++++++++++++++---- testsuite/MDAnalysisTests/coordinates/base.py | 24 ++++++++++++++++- .../MDAnalysisTests/coordinates/test_dcd.py | 6 ----- .../coordinates/test_memory.py | 14 ++++++++++ 7 files changed, 96 insertions(+), 18 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index eddb1fb8eac..4551124635c 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -59,6 +59,8 @@ Changes `analysis.nucleicacids.WatsonCrickDist.results.distances` (Issue #3720, PR #3735) Deprecations + * The `asel` argument to timeseries will be renamed to `atomgroup` in 3.0.0. + (Issue #3911) * MDAnalysis.lib.util is deprecated and will be removed in version 3.0 (Issue #3649) * The TRZ reader & writer are deprecated and will be removed in version 3.0 diff --git a/package/MDAnalysis/coordinates/DCD.py b/package/MDAnalysis/coordinates/DCD.py index 305abd74fcf..b28c12ca4c2 100644 --- a/package/MDAnalysis/coordinates/DCD.py +++ b/package/MDAnalysis/coordinates/DCD.py @@ -278,6 +278,7 @@ def dt(self): def timeseries(self, asel=None, + atomgroup=None, start=None, stop=None, step=None, @@ -290,6 +291,12 @@ def timeseries(self, The :class:`~MDAnalysis.core.groups.AtomGroup` to read the coordinates from. Defaults to None, in which case the full set of coordinate data is returned. + + .. deprecated:: 2.7.0 + asel argument will be renamed to atomgroup in 3.0.0 + + atomgroup: AtomGroup (optional) + Same as `asel`, will replace `asel` in 3.0.0 start : int (optional) Begin reading the trajectory at frame index `start` (where 0 is the index of the first frame in the trajectory); the default ``None`` @@ -315,14 +322,22 @@ def timeseries(self, ValueError now raised instead of NoDataError for empty input AtomGroup """ + if asel is not None: + warnings.warn( + "asel argument to timeseries will be renamed to" + "'atomgroup' in 3.0, see #3911", + category=DeprecationWarning) + if atomgroup: + raise ValueError("Cannot provide both asel and atomgroup kwargs") + atomgroup = asel start, stop, step = self.check_slice_indices(start, stop, step) - if asel is not None: - if len(asel) == 0: + if atomgroup is not None: + if len(atomgroup) == 0: raise ValueError( "Timeseries requires at least one atom to analyze") - atom_numbers = list(asel.indices) + atom_numbers = list(atomgroup.indices) else: atom_numbers = list(range(self.n_atoms)) diff --git a/package/MDAnalysis/coordinates/base.py b/package/MDAnalysis/coordinates/base.py index abe8d74dff6..ce825f243fc 100644 --- a/package/MDAnalysis/coordinates/base.py +++ b/package/MDAnalysis/coordinates/base.py @@ -987,6 +987,7 @@ def __repr__(self): )) def timeseries(self, asel: Optional['AtomGroup']=None, + atomgroup: Optional['Atomgroup']=None, start: Optional[int]=None, stop: Optional[int]=None, step: Optional[int]=None, order: Optional[str]='fac') -> np.ndarray: @@ -998,6 +999,12 @@ def timeseries(self, asel: Optional['AtomGroup']=None, The :class:`~MDAnalysis.core.groups.AtomGroup` to read the coordinates from. Defaults to ``None``, in which case the full set of coordinate data is returned. + + .. deprecated:: 2.7.0 + asel argument will be renamed to atomgroup in 3.0.0 + + atomgroup: AtomGroup (optional) + Same as `asel`, will replace `asel` in 3.0.0 start : int (optional) Begin reading the trajectory at frame index `start` (where 0 is the index of the first frame in the trajectory); the default @@ -1023,14 +1030,22 @@ def timeseries(self, asel: Optional['AtomGroup']=None, .. versionadded:: 2.4.0 """ + if asel is not None: + warnings.warn( + "asel argument to timeseries will be renamed to" + "'atomgroup' in 3.0, see #3911", + category=DeprecationWarning) + if atomgroup: + raise ValueError("Cannot provide both asel and atomgroup kwargs") + atomgroup = asel start, stop, step = self.check_slice_indices(start, stop, step) nframes = len(range(start, stop, step)) - if asel is not None: - if len(asel) == 0: + if atomgroup is not None: + if len(atomgroup) == 0: raise ValueError( "Timeseries requires at least one atom to analyze") - atom_numbers = asel.indices + atom_numbers = atomgroup.indices natoms = len(atom_numbers) else: natoms = self.n_atoms diff --git a/package/MDAnalysis/coordinates/memory.py b/package/MDAnalysis/coordinates/memory.py index 8c2b459128d..d2521b9a21b 100644 --- a/package/MDAnalysis/coordinates/memory.py +++ b/package/MDAnalysis/coordinates/memory.py @@ -488,7 +488,7 @@ def _reopen(self): self.ts.frame = -1 self.ts.time = -1 - def timeseries(self, asel=None, start=0, stop=-1, step=1, order='afc'): + def timeseries(self, asel=None, atomgroup=None, start=0, stop=-1, step=1, order='afc'): """Return a subset of coordinate data for an AtomGroup in desired column order. If no selection is given, it will return a view of the underlying array, while a copy is returned otherwise. @@ -500,6 +500,12 @@ def timeseries(self, asel=None, start=0, stop=-1, step=1, order='afc'): coordinate data is returned. Note that in this case, a view of the underlying numpy array is returned, while a copy of the data is returned whenever `asel` is different from ``None``. + + .. deprecated:: 2.7.0 + asel argument will be renamed to atomgroup in 3.0.0 + + atomgroup: AtomGroup (optional) + Same as `asel`, will replace `asel` in 3.0.0 start : int (optional) the start trajectory frame stop : int (optional) @@ -525,6 +531,16 @@ def timeseries(self, asel=None, start=0, stop=-1, step=1, order='afc'): ValueError now raised instead of NoDataError for empty input AtomGroup """ + if asel is not None: + warnings.warn( + "asel argument to timeseries will be renamed to" + "'atomgroup' in 3.0, see #3911", + category=DeprecationWarning) + if atomgroup: + raise ValueError("Cannot provide both asel and atomgroup kwargs") + atomgroup = asel + + if stop != -1: warnings.warn("MemoryReader.timeseries inclusive `stop` " "indexing will be removed in 3.0 in favour of exclusive " @@ -556,13 +572,13 @@ def timeseries(self, asel=None, start=0, stop=-1, step=1, order='afc'): [slice(None)] * (2-f_index)) # Return a view if either: - # 1) asel is None - # 2) asel corresponds to the selection of all atoms. + # 1) atomgroup is None + # 2) atomgroup corresponds to the selection of all atoms. array = array[tuple(basic_slice)] - if (asel is None or asel is asel.universe.atoms): + if (atomgroup is None or atomgroup is atomgroup.universe.atoms): return array else: - if len(asel) == 0: + if len(atomgroup) == 0: raise ValueError("Timeseries requires at least one atom " "to analyze") # If selection is specified, return a copy diff --git a/testsuite/MDAnalysisTests/coordinates/base.py b/testsuite/MDAnalysisTests/coordinates/base.py index 67a62fb9c37..a2a131bbbde 100644 --- a/testsuite/MDAnalysisTests/coordinates/base.py +++ b/testsuite/MDAnalysisTests/coordinates/base.py @@ -499,7 +499,29 @@ def test_timeseries_empty_asel(self, reader): 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) + reader.timeseries(asel=atoms) + + def test_timeseries_empty_atomgroup(self, reader): + 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(atomgroup=atoms) + + def test_timeseries_asel_warns_deprecation(self, reader): + atoms = mda.Universe(reader.filename).select_atoms("index 1") + with pytest.warns(DeprecationWarning, match="asel argument to"): + timeseries = reader.timeseries(asel=atoms, order='fac') + + def test_timeseries_atomgroup(self, reader): + atoms = mda.Universe(reader.filename).select_atoms("index 1") + timeseries = reader.timeseries(atomgroup=atoms, order='fac') + + def test_timeseries_atomgroup_asel_mutex(self, reader): + atoms = mda.Universe(reader.filename).select_atoms("index 1") + with pytest.raises(ValueError, match="Cannot provide both"): + timeseries = reader.timeseries(atomgroup=atoms, asel=atoms, order='fac') + class MultiframeReaderTest(BaseReaderTest): def test_last_frame(self, ref, reader): diff --git a/testsuite/MDAnalysisTests/coordinates/test_dcd.py b/testsuite/MDAnalysisTests/coordinates/test_dcd.py index fc0255f8127..e6fa4dfb3e3 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_dcd.py +++ b/testsuite/MDAnalysisTests/coordinates/test_dcd.py @@ -231,12 +231,6 @@ def test_timeseries_atomindices(indices, universe_dcd): assert_array_almost_equal(xyz, allframes[indices]) -def test_timeseries_empty_selection(universe_dcd): - with pytest.raises(ValueError): - asel = universe_dcd.select_atoms('name FOO') - universe_dcd.trajectory.timeseries(asel=asel) - - def test_reader_set_dt(): dt = 100 frame = 3 diff --git a/testsuite/MDAnalysisTests/coordinates/test_memory.py b/testsuite/MDAnalysisTests/coordinates/test_memory.py index 6e516714018..b77006cc04e 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_memory.py +++ b/testsuite/MDAnalysisTests/coordinates/test_memory.py @@ -202,6 +202,20 @@ def test_timeseries_warns_deprecation(self, reader): "inclusive"): reader.timeseries(start=0, stop=3, step=1) + def test_timeseries_asel_warns_deprecation(self, ref, reader): + selection = ref.universe.atoms + with pytest.warns(DeprecationWarning, match="asel argument to"): + reader.timeseries(asel=selection) + + def test_timeseries_atomgroup(self, ref, reader): + selection = ref.universe.atoms + reader.timeseries(atomgroup=selection) + + def test_timeseries_atomgroup_asel_mutex(self, ref, reader): + selection = ref.universe.atoms + with pytest.raises(ValueError, match="Cannot provide both"): + reader.timeseries(atomgroup=selection, asel=selection) + class TestMemoryReaderVelsForces(object): @staticmethod