From f7815481bf6edeb62a01dda5af870f8822e78ff3 Mon Sep 17 00:00:00 2001 From: Philip Loche Date: Thu, 23 Mar 2023 16:36:29 +0100 Subject: [PATCH 1/8] Add a `AnalayisCollection` class --- package/CHANGELOG | 4 +- package/MDAnalysis/analysis/base.py | 183 +++++++++++++++--- .../MDAnalysisTests/analysis/test_base.py | 74 ++++++- 3 files changed, 229 insertions(+), 32 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 1de34d5be7..ed2cbc1af5 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -16,7 +16,7 @@ The rules for this file: ??/??/?? IAlibay, pgbarletta, mglagolev, hmacdope, manuel.nuno.melo, chrispfae, ooprathamm, MeetB7, BFedder, v-parmar, MoSchaeffler, jbarnoud, jandom, - xhgchen, jaclark5, DrDomenicoMarson + xhgchen, jaclark5, DrDomenicoMarson, PicoCentauri * 2.5.0 Fixes @@ -34,6 +34,8 @@ Fixes (Issue #3336) Enhancements + * Add a `AnalayisCollection` class to perform multiple analysis on the same + trajectory (#3569, PR #4017). * Added AtomGroup TopologyAttr to calculate gyration moments (Issue #3904, PR #3905) * Add support for TPR files produced by Gromacs 2023 (Issue #4047) diff --git a/package/MDAnalysis/analysis/base.py b/package/MDAnalysis/analysis/base.py index 266ae68a02..a0292deb52 100644 --- a/package/MDAnalysis/analysis/base.py +++ b/package/MDAnalysis/analysis/base.py @@ -220,7 +220,152 @@ def __setstate__(self, state): self.data = state -class AnalysisBase(object): +class AnalysisCollection(object): + """ + Class for running a collection of analysis classes on a single trajectory. + + Running a collection of analysis with ``AnalysisCollection`` can result in + a speedup compared to running the individual object since here we only + perform the trajectory looping once. + + The class assumes that each analysis is a child of + :class:`MDAnalysis.analysis.base.AnalysisBase`. Additionally, + the trajectory of all ``analysis_objects`` must be same. + + By default it is ensured that all analyisis objects use the + *same original* timestep and not an altered one from a previous analysis + object. This behaviour can be changed with the ``reset_timestep`` parameter + of the :meth:`MDAnalysis.analysis.base.AnalysisCollection.run` method. + + Parameters + ---------- + *analysis_objects : tuple + List of analysis classes to run on the same trajectory. + + Raises + ------ + AttributeError + If the provided ``analysis_objects`` do not have the same trajectory. + AttributeError + If an ``analysis_object`` is not a child of + :class:`MDAnalysis.analysis.base.AnalysisBase`. + + Example + ------- + .. code-block:: python + + import MDAnalysis as mda + from MDAnalysis.analysis.rdf import InterRDF + from MDAnalysis.analysis.base import AnalysisCollection + from MDAnalysisTests.datafiles import TPR, XTC + + u = mda.Universe(TPR, XTC) + + # Select atoms + O = u.select_atoms('name O') + H = u.select_atoms('name H') + + # Create individual analysis objects + rdf_OO = InterRDF(O, O) + rdf_OH = InterRDF(O, H) + + # Create collection for common trajectory + collection = AnalysisCollection(rdf_OO, rdf_OH) + + # Run the collected analysis + collection.run(start=0, end=100, step=10) + + # Results are stored in indivial objects + print(rdf_OO.results) + print(rdf_OH.results) + + .. versionadded:: 2.5.0 + """ + def __init__(self, *analysis_objects): + for analysis_object in analysis_objects: + if analysis_objects[0]._trajectory != analysis_object._trajectory: + raise ValueError("`analysis_objects` do not have the same " + "trajectory.") + if not isinstance(analysis_object, AnalysisBase): + raise AttributeError(f"Analysis object {analysis_object} is " + "not a child of `AnalysisBase`.") + + self._analysis_objects = analysis_objects + + def run(self, start=None, stop=None, step=None, frames=None, + verbose=None, reset_timestep=True): + """Perform the calculation + + Parameters + ---------- + start : int, optional + start frame of analysis + stop : int, optional + stop frame of analysis + step : int, optional + number of frames to skip between each analysed frame + frames : array_like, optional + array of integers or booleans to slice trajectory; ``frames`` can + only be used *instead* of ``start``, ``stop``, and ``step``. Setting + *both* `frames` and at least one of ``start``, ``stop``, ``step`` to a + non-default value will raise a :exc:``ValueError``. + verbose : bool, optional + Turn on verbosity + reset_timestep : bool + Reset the timestep object after for each ``analysis_object``. + Setting this to ``False`` can be useful if an ``analysis_object`` + is performing a trajectory manipulation which is also useful for the + subsequent ``analysis_objects`` e.g. unwrapping of molecules. + """ + + # Ensure compatibility with API of version 0.15.0 + if not hasattr(self, "_analysis_objects"): + self._analysis_objects = (self, ) + + logger.info("Choosing frames to analyze") + # if verbose unchanged, use class default + verbose = getattr(self, '_verbose', + False) if verbose is None else verbose + + logger.info("Starting preparation") + + for analysis_object in self._analysis_objects: + analysis_object._setup_frames(analysis_object._trajectory, + start=start, + stop=stop, + step=step, + frames=frames) + analysis_object._prepare() + + logger.info("Starting analysis loop over" + f"{self._analysis_objects[0].n_frames} trajectory frames") + + for i, ts in enumerate(ProgressBar( + self._analysis_objects[0]._sliced_trajectory, + verbose=verbose)): + + if reset_timestep: + ts_original = ts.copy() + + for analysis_object in self._analysis_objects: + analysis_object._frame_index = i + analysis_object._ts = ts + analysis_object.frames[i] = ts.frame + analysis_object.times[i] = ts.time + analysis_object._single_frame() + + if reset_timestep: + ts = ts_original + + logger.info("Finishing up") + + for analysis_object in self._analysis_objects: + analysis_object._conclude() + + return self + + +class AnalysisBase(AnalysisCollection): r"""Base class for defining multi-frame analysis The class is designed as a template for creating multi-frame analyses. @@ -316,6 +461,7 @@ def __init__(self, trajectory, verbose=False, **kwargs): self._trajectory = trajectory self._verbose = verbose self.results = Results() + super(AnalysisBase, self).__init__(self) def _setup_frames(self, trajectory, start=None, stop=None, step=None, frames=None): @@ -402,10 +548,10 @@ def run(self, start=None, stop=None, step=None, frames=None, step : int, optional number of frames to skip between each analysed frame frames : array_like, optional - array of integers or booleans to slice trajectory; `frames` can - only be used *instead* of `start`, `stop`, and `step`. Setting - *both* `frames` and at least one of `start`, `stop`, `step` to a - non-default value will raise a :exc:`ValueError`. + array of integers or booleans to slice trajectory; ``frames`` can + only be used *instead* of ``start``, ``stop``, and ``step``. Setting + *both* `frames` and at least one of ``start``, ``stop``, ``step`` to a + non-default value will raise a :exc:``ValueError``. .. versionadded:: 2.2.0 @@ -418,28 +564,11 @@ def run(self, start=None, stop=None, step=None, frames=None, frame indices in the `frames` keyword argument. """ - logger.info("Choosing frames to analyze") - # if verbose unchanged, use class default - verbose = getattr(self, '_verbose', - False) if verbose is None else verbose - - self._setup_frames(self._trajectory, start=start, stop=stop, - step=step, frames=frames) - logger.info("Starting preparation") - self._prepare() - logger.info("Starting analysis loop over %d trajectory frames", - self.n_frames) - for i, ts in enumerate(ProgressBar( - self._sliced_trajectory, - verbose=verbose)): - self._frame_index = i - self._ts = ts - self.frames[i] = ts.frame - self.times[i] = ts.time - self._single_frame() - logger.info("Finishing up") - self._conclude() - return self + return super(AnalysisBase, self).run(start=start, + stop=stop, + step=step, + frames=frames, + verbose=verbose) class AnalysisFromFunction(AnalysisBase): diff --git a/testsuite/MDAnalysisTests/analysis/test_base.py b/testsuite/MDAnalysisTests/analysis/test_base.py index 09b7b158f0..acb8ac33f2 100644 --- a/testsuite/MDAnalysisTests/analysis/test_base.py +++ b/testsuite/MDAnalysisTests/analysis/test_base.py @@ -27,10 +27,11 @@ import numpy as np -from numpy.testing import assert_equal, assert_almost_equal +from numpy.testing import assert_equal, assert_allclose import MDAnalysis as mda from MDAnalysis.analysis import base +from MDAnalysis.analysis.rdf import InterRDF from MDAnalysisTests.datafiles import PSF, DCD, TPR, XTC from MDAnalysisTests.util import no_deprecated_call @@ -147,6 +148,71 @@ def test_different_instances(self, results): assert new_results.data is not results.data +class TestAnalysisCollection: + @pytest.fixture + def universe(self): + return mda.Universe(TPR, XTC) + + def test_run(self, universe): + O = universe.select_atoms('name O') + H = universe.select_atoms('name H') + + rdf_OO = InterRDF(O, O) + rdf_OH = InterRDF(O, H) + + collection = base.AnalysisCollection(rdf_OO, rdf_OH) + collection.run(start=0, stop=100, step=10) + + assert rdf_OO.results is not None + assert rdf_OH.results is not None + + @pytest.mark.parametrize("reset_timestep", [True, False]) + def test_trajectory_manipulation(self, universe, reset_timestep): + + class CustomAnalysis(base.AnalysisBase): + """Custom class that is shifting positions in every step by 10.""" + def __init__(self, trajectory): + self._trajectory = trajectory + + def _prepare(self): + pass + + def _single_frame(self): + self._ts.positions += 10 + self.ref_pos = self._ts.positions.copy()[0,0] + + ana_1 = CustomAnalysis(universe.trajectory) + ana_2 = CustomAnalysis(universe.trajectory) + + collection = base.AnalysisCollection(ana_1, ana_2) + + collection.run(frames=[0], reset_timestep=reset_timestep) + + if reset_timestep: + assert ana_2.ref_pos == ana_1.ref_pos + else: + assert_allclose(ana_2.ref_pos, ana_1.ref_pos + 10.) + + def test_no_trajectory_manipulation(self): + pass + + def test_inconsistent_trajectory(self, universe): + v = mda.Universe(TPR, XTC) + + with pytest.raises(ValueError, match="`analysis_objects` do not have the same"): + base.AnalysisCollection(InterRDF(universe.atoms, universe.atoms), + InterRDF(v.atoms, v.atoms)) + + def test_no_base_child(self, universe): + class CustomAnalysis: + def __init__(self, trajectory): + self._trajectory = trajectory + + # Create collection for common trajectory loop with inconsistent trajectory + with pytest.raises(AttributeError, match="not a child of `AnalysisBa"): + base.AnalysisCollection(CustomAnalysis(universe.trajectory)) + + class FrameAnalysis(base.AnalysisBase): """Just grabs frame numbers of frames it goes over""" @@ -194,7 +260,7 @@ def test_start_stop_step(u, run_kwargs, frames): assert an.n_frames == len(frames) assert_equal(an.found_frames, frames) assert_equal(an.frames, frames, err_msg=FRAMES_ERR) - assert_almost_equal(an.times, frames+1, decimal=4, err_msg=TIMES_ERR) + assert_allclose(an.times, frames+1, rtol=1e-4, err_msg=TIMES_ERR) @pytest.mark.parametrize('run_kwargs, frames', [ @@ -251,7 +317,7 @@ def test_frames_times(): assert an.n_frames == len(frames) assert_equal(an.found_frames, frames) assert_equal(an.frames, frames, err_msg=FRAMES_ERR) - assert_almost_equal(an.times, frames*100, decimal=4, err_msg=TIMES_ERR) + assert_allclose(an.times, frames*100, rtol=1e-4, err_msg=TIMES_ERR) def test_verbose(u): @@ -366,7 +432,7 @@ def test_AnalysisFromFunction_args_content(u): ans = base.AnalysisFromFunction(mass_xyz, protein, another, masses) assert len(ans.args) == 3 result = np.sum(ans.run().results.timeseries) - assert_almost_equal(result, -317054.67757345125, decimal=6) + assert_allclose(result, -317054.67757345125, rtol=1e-6) assert (ans.args[0] is protein) and (ans.args[1] is another) assert ans._trajectory is protein.universe.trajectory From b58b2943326ebc0cff237fecb76b9ed5ad28ea29 Mon Sep 17 00:00:00 2001 From: Philip Loche Date: Mon, 30 Jan 2023 12:13:50 +0100 Subject: [PATCH 2/8] try make linter happy --- package/MDAnalysis/analysis/base.py | 71 +++++++++++-------- .../MDAnalysisTests/analysis/test_base.py | 23 +++--- 2 files changed, 52 insertions(+), 42 deletions(-) diff --git a/package/MDAnalysis/analysis/base.py b/package/MDAnalysis/analysis/base.py index a0292deb52..c709fe6e6a 100644 --- a/package/MDAnalysis/analysis/base.py +++ b/package/MDAnalysis/analysis/base.py @@ -225,14 +225,14 @@ class AnalysisCollection(object): Class for running a collection of analysis classes on a single trajectory. Running a collection of analysis with ``AnalysisCollection`` can result in - a speedup compared to running the individual object since here we only + a speedup compared to running the individual object since here we only perform the trajectory looping once. The class assumes that each analysis is a child of :class:`MDAnalysis.analysis.base.AnalysisBase`. Additionally, the trajectory of all ``analysis_objects`` must be same. - By default it is ensured that all analyisis objects use the + By default it is ensured that all analyisis objects use the *same original* timestep and not an altered one from a previous analysis object. This behaviour can be changed with the ``reset_timestep`` parameter of the :meth:`MDAnalysis.analysis.base.AnalysisCollection.run` method. @@ -262,12 +262,12 @@ class AnalysisCollection(object): u = mda.Universe(TPR, XTC) # Select atoms - O = u.select_atoms('name O') - H = u.select_atoms('name H') + ag_O = u.select_atoms("name O") + ag_H = u.select_atoms("name H") # Create individual analysis objects - rdf_OO = InterRDF(O, O) - rdf_OH = InterRDF(O, H) + rdf_OO = InterRDF(ag_O, ag_O) + rdf_OH = InterRDF(ag_H, ag_H) # Create collection for common trajectory collection = AnalysisCollection(rdf_OO, rdf_OH) @@ -281,19 +281,28 @@ class AnalysisCollection(object): .. versionadded:: 2.5.0 """ + def __init__(self, *analysis_objects): for analysis_object in analysis_objects: if analysis_objects[0]._trajectory != analysis_object._trajectory: - raise ValueError("`analysis_objects` do not have the same " - "trajectory.") + raise ValueError("`analysis_objects` do not have the same trajectory.") if not isinstance(analysis_object, AnalysisBase): - raise AttributeError(f"Analysis object {analysis_object} is " - "not a child of `AnalysisBase`.") + raise AttributeError( + f"Analysis object {analysis_object} is " + "not a child of `AnalysisBase`." + ) self._analysis_objects = analysis_objects - def run(self, start=None, stop=None, step=None, frames=None, - verbose=None, reset_timestep=True): + def run( + self, + start=None, + stop=None, + step=None, + frames=None, + verbose=None, + reset_timestep=True, + ): """Perform the calculation Parameters @@ -320,30 +329,32 @@ def run(self, start=None, stop=None, step=None, frames=None, # Ensure compatibility with API of version 0.15.0 if not hasattr(self, "_analysis_objects"): - self._analysis_objects = (self, ) + self._analysis_objects = (self,) logger.info("Choosing frames to analyze") # if verbose unchanged, use class default - verbose = getattr(self, '_verbose', - False) if verbose is None else verbose + verbose = getattr(self, "_verbose", False) if verbose is None else verbose logger.info("Starting preparation") for analysis_object in self._analysis_objects: - analysis_object._setup_frames(analysis_object._trajectory, - start=start, - stop=stop, - step=step, - frames=frames) + analysis_object._setup_frames( + analysis_object._trajectory, + start=start, + stop=stop, + step=step, + frames=frames, + ) analysis_object._prepare() - logger.info("Starting analysis loop over" - f"{self._analysis_objects[0].n_frames} trajectory frames") - - for i, ts in enumerate(ProgressBar( - self._analysis_objects[0]._sliced_trajectory, - verbose=verbose)): + logger.info( + f"Starting analysis loop over {self._analysis_objects[0].n_frames} " + "trajectory frames." + ) + for i, ts in enumerate( + ProgressBar(self._analysis_objects[0]._sliced_trajectory, verbose=verbose) + ): if reset_timestep: ts_original = ts.copy() @@ -564,11 +575,9 @@ def run(self, start=None, stop=None, step=None, frames=None, frame indices in the `frames` keyword argument. """ - return super(AnalysisBase, self).run(start=start, - stop=stop, - step=step, - frames=frames, - verbose=verbose) + return super(AnalysisBase, self).run( + start=start, stop=stop, step=step, frames=frames, verbose=verbose + ) class AnalysisFromFunction(AnalysisBase): diff --git a/testsuite/MDAnalysisTests/analysis/test_base.py b/testsuite/MDAnalysisTests/analysis/test_base.py index acb8ac33f2..a4ecd7c1a5 100644 --- a/testsuite/MDAnalysisTests/analysis/test_base.py +++ b/testsuite/MDAnalysisTests/analysis/test_base.py @@ -154,11 +154,11 @@ def universe(self): return mda.Universe(TPR, XTC) def test_run(self, universe): - O = universe.select_atoms('name O') - H = universe.select_atoms('name H') + ag_O = universe.select_atoms("name O") + ag_H = universe.select_atoms("name H") - rdf_OO = InterRDF(O, O) - rdf_OH = InterRDF(O, H) + rdf_OO = InterRDF(ag_O, ag_O) + rdf_OH = InterRDF(ag_O, ag_H) collection = base.AnalysisCollection(rdf_OO, rdf_OH) collection.run(start=0, stop=100, step=10) @@ -168,9 +168,9 @@ def test_run(self, universe): @pytest.mark.parametrize("reset_timestep", [True, False]) def test_trajectory_manipulation(self, universe, reset_timestep): - class CustomAnalysis(base.AnalysisBase): """Custom class that is shifting positions in every step by 10.""" + def __init__(self, trajectory): self._trajectory = trajectory @@ -179,7 +179,7 @@ def _prepare(self): def _single_frame(self): self._ts.positions += 10 - self.ref_pos = self._ts.positions.copy()[0,0] + self.ref_pos = self._ts.positions.copy()[0, 0] ana_1 = CustomAnalysis(universe.trajectory) ana_2 = CustomAnalysis(universe.trajectory) @@ -191,7 +191,7 @@ def _single_frame(self): if reset_timestep: assert ana_2.ref_pos == ana_1.ref_pos else: - assert_allclose(ana_2.ref_pos, ana_1.ref_pos + 10.) + assert_allclose(ana_2.ref_pos, ana_1.ref_pos + 10) def test_no_trajectory_manipulation(self): pass @@ -200,8 +200,9 @@ def test_inconsistent_trajectory(self, universe): v = mda.Universe(TPR, XTC) with pytest.raises(ValueError, match="`analysis_objects` do not have the same"): - base.AnalysisCollection(InterRDF(universe.atoms, universe.atoms), - InterRDF(v.atoms, v.atoms)) + base.AnalysisCollection( + InterRDF(universe.atoms, universe.atoms), InterRDF(v.atoms, v.atoms) + ) def test_no_base_child(self, universe): class CustomAnalysis: @@ -260,7 +261,7 @@ def test_start_stop_step(u, run_kwargs, frames): assert an.n_frames == len(frames) assert_equal(an.found_frames, frames) assert_equal(an.frames, frames, err_msg=FRAMES_ERR) - assert_allclose(an.times, frames+1, rtol=1e-4, err_msg=TIMES_ERR) + assert_allclose(an.times, frames + 1, rtol=1e-4, err_msg=TIMES_ERR) @pytest.mark.parametrize('run_kwargs, frames', [ @@ -317,7 +318,7 @@ def test_frames_times(): assert an.n_frames == len(frames) assert_equal(an.found_frames, frames) assert_equal(an.frames, frames, err_msg=FRAMES_ERR) - assert_allclose(an.times, frames*100, rtol=1e-4, err_msg=TIMES_ERR) + assert_allclose(an.times, frames * 100, rtol=1e-4, err_msg=TIMES_ERR) def test_verbose(u): From b71e9452ec931b63455ee117f9e18567b295e8fc Mon Sep 17 00:00:00 2001 From: Philip Loche Date: Mon, 30 Jan 2023 16:24:13 +0100 Subject: [PATCH 3/8] proofread --- package/CHANGELOG | 2 +- package/MDAnalysis/analysis/base.py | 38 ++++++++++--------- .../MDAnalysisTests/analysis/test_base.py | 3 -- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index ed2cbc1af5..89b5984e10 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -34,7 +34,7 @@ Fixes (Issue #3336) Enhancements - * Add a `AnalayisCollection` class to perform multiple analysis on the same + * Add an `AnalayisCollection` class to perform multiple analysis on the same trajectory (#3569, PR #4017). * Added AtomGroup TopologyAttr to calculate gyration moments (Issue #3904, PR #3905) diff --git a/package/MDAnalysis/analysis/base.py b/package/MDAnalysis/analysis/base.py index c709fe6e6a..4b5f8822e9 100644 --- a/package/MDAnalysis/analysis/base.py +++ b/package/MDAnalysis/analysis/base.py @@ -230,11 +230,11 @@ class AnalysisCollection(object): The class assumes that each analysis is a child of :class:`MDAnalysis.analysis.base.AnalysisBase`. Additionally, - the trajectory of all ``analysis_objects`` must be same. + the trajectory of all ``analysis_objects`` must be the same. - By default it is ensured that all analyisis objects use the + By default, it is ensured that all analysis objects use the *same original* timestep and not an altered one from a previous analysis - object. This behaviour can be changed with the ``reset_timestep`` parameter + object. This behavior can be changed with the ``reset_timestep`` parameter of the :meth:`MDAnalysis.analysis.base.AnalysisCollection.run` method. Parameters @@ -245,7 +245,7 @@ class AnalysisCollection(object): Raises ------ AttributeError - If the provided ``analysis_objects`` do not have the same trajectory. + If all the provided ``analysis_objects`` do not have the same trajectory. AttributeError If an ``analysis_object`` is not a child of :class:`MDAnalysis.analysis.base.AnalysisBase`. @@ -269,17 +269,19 @@ class AnalysisCollection(object): rdf_OO = InterRDF(ag_O, ag_O) rdf_OH = InterRDF(ag_H, ag_H) - # Create collection for common trajectory + # Create a collection for common trajectory collection = AnalysisCollection(rdf_OO, rdf_OH) # Run the collected analysis collection.run(start=0, end=100, step=10) - # Results are stored in indivial objects + # Results are stored in individual objects print(rdf_OO.results) print(rdf_OH.results) + .. versionadded:: 2.5.0 + """ def __init__(self, *analysis_objects): @@ -314,13 +316,13 @@ def run( step : int, optional number of frames to skip between each analysed frame frames : array_like, optional - array of integers or booleans to slice trajectory; ``frames`` can - only be used *instead* of ``start``, ``stop``, and ``step``. Setting - *both* `frames` and at least one of ``start``, ``stop``, ``step`` to a - non-default value will raise a :exc:``ValueError``. + array of integers or booleans to slice trajectory; `frames` can + only be used *instead* of `start`, `stop`, and `step`. Setting + *both* `frames` and at least one of `start`, `stop`, `step` to a + non-default value will raise a :exc:`ValueError`. verbose : bool, optional Turn on verbosity - reset_timestep : bool + reset_timestep : bool, optional Reset the timestep object after for each ``analysis_object``. Setting this to ``False`` can be useful if an ``analysis_object`` is performing a trajectory manipulation which is also useful for the @@ -490,8 +492,10 @@ def _setup_frames(self, trajectory, start=None, stop=None, step=None, step : int, optional number of frames to skip between each analysed frame frames : array_like, optional - array of integers or booleans to slice trajectory; cannot be - combined with `start`, `stop`, `step` + array of integers or booleans to slice trajectory; `frames` can + only be used *instead* of `start`, `stop`, and `step`. Setting + *both* `frames` and at least one of `start`, `stop`, `step` to a + non-default value will raise a :exc:`ValueError`. .. versionadded:: 2.2.0 @@ -559,10 +563,10 @@ def run(self, start=None, stop=None, step=None, frames=None, step : int, optional number of frames to skip between each analysed frame frames : array_like, optional - array of integers or booleans to slice trajectory; ``frames`` can - only be used *instead* of ``start``, ``stop``, and ``step``. Setting - *both* `frames` and at least one of ``start``, ``stop``, ``step`` to a - non-default value will raise a :exc:``ValueError``. + array of integers or booleans to slice trajectory; `frames` can + only be used *instead* of `start`, `stop`, and `step`. Setting + *both* `frames` and at least one of `start`, `stop`, `step` to a + non-default value will raise a :exc:`ValueError`. .. versionadded:: 2.2.0 diff --git a/testsuite/MDAnalysisTests/analysis/test_base.py b/testsuite/MDAnalysisTests/analysis/test_base.py index a4ecd7c1a5..b17fb616d1 100644 --- a/testsuite/MDAnalysisTests/analysis/test_base.py +++ b/testsuite/MDAnalysisTests/analysis/test_base.py @@ -193,9 +193,6 @@ def _single_frame(self): else: assert_allclose(ana_2.ref_pos, ana_1.ref_pos + 10) - def test_no_trajectory_manipulation(self): - pass - def test_inconsistent_trajectory(self, universe): v = mda.Universe(TPR, XTC) From a1b6cdf41e23ae838f125c9159ca07615dc1b278 Mon Sep 17 00:00:00 2001 From: Philip Loche Date: Thu, 16 Feb 2023 16:29:46 +0100 Subject: [PATCH 4/8] Fix typos and improve docs and code comments --- package/MDAnalysis/analysis/base.py | 65 ++++++++++++------- .../MDAnalysisTests/analysis/test_base.py | 4 +- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/package/MDAnalysis/analysis/base.py b/package/MDAnalysis/analysis/base.py index 4b5f8822e9..35b6c2b223 100644 --- a/package/MDAnalysis/analysis/base.py +++ b/package/MDAnalysis/analysis/base.py @@ -89,6 +89,10 @@ plt.xlabel("time (ps)") plt.ylabel("RMSD (Å)") +If you want to run two or more different analyses on the same trajectory you +can also efficently combine them using the +:class:`MDAnalysis.analysis.base.AnalysisCollection` class. + Writing new analysis tools -------------------------- @@ -224,28 +228,32 @@ class AnalysisCollection(object): """ Class for running a collection of analysis classes on a single trajectory. - Running a collection of analysis with ``AnalysisCollection`` can result in - a speedup compared to running the individual object since here we only - perform the trajectory looping once. + Running a collection of analyses with ``AnalysisCollection`` can result in + a speedup compared to running the individual analyses since the trajectory + loop ins only performed once. The class assumes that each analysis is a child of - :class:`MDAnalysis.analysis.base.AnalysisBase`. Additionally, - the trajectory of all ``analysis_objects`` must be the same. + :class:`MDAnalysis.analysis.base.AnalysisBase`. Additionally, the + trajectory of all `analysis_instances` must be the same. - By default, it is ensured that all analysis objects use the + By default, it is ensured that all analysis instances use the *same original* timestep and not an altered one from a previous analysis - object. This behavior can be changed with the ``reset_timestep`` parameter + object. This behavior can be changed with the `reset_timestep` parameter of the :meth:`MDAnalysis.analysis.base.AnalysisCollection.run` method. + Changing the default behaviour of `reset_timestep` might be useful to + avoid that subsequent analysis instances have to perform the same + transformation on the timestep. Parameters ---------- - *analysis_objects : tuple + *analysis_instances : tuple List of analysis classes to run on the same trajectory. Raises ------ AttributeError - If all the provided ``analysis_objects`` do not have the same trajectory. + If all the provided ``analysis_instances`` do not work on the same + trajectory. AttributeError If an ``analysis_object`` is not a child of :class:`MDAnalysis.analysis.base.AnalysisBase`. @@ -265,7 +273,7 @@ class AnalysisCollection(object): ag_O = u.select_atoms("name O") ag_H = u.select_atoms("name H") - # Create individual analysis objects + # Create individual analysis instances rdf_OO = InterRDF(ag_O, ag_O) rdf_OH = InterRDF(ag_H, ag_H) @@ -273,9 +281,9 @@ class AnalysisCollection(object): collection = AnalysisCollection(rdf_OO, rdf_OH) # Run the collected analysis - collection.run(start=0, end=100, step=10) + collection.run(start=0, stop=100, step=10) - # Results are stored in individual objects + # Results are stored in the individual instances print(rdf_OO.results) print(rdf_OH.results) @@ -284,17 +292,19 @@ class AnalysisCollection(object): """ - def __init__(self, *analysis_objects): - for analysis_object in analysis_objects: - if analysis_objects[0]._trajectory != analysis_object._trajectory: - raise ValueError("`analysis_objects` do not have the same trajectory.") + def __init__(self, *analysis_instances): + for analysis_object in analysis_instances: + if analysis_instances[0]._trajectory != analysis_object._trajectory: + raise ValueError( + "`analysis_instances` do not have the same trajectory." + ) if not isinstance(analysis_object, AnalysisBase): raise AttributeError( f"Analysis object {analysis_object} is " "not a child of `AnalysisBase`." ) - self._analysis_objects = analysis_objects + self._analysis_instances = analysis_instances def run( self, @@ -326,12 +336,12 @@ def run( Reset the timestep object after for each ``analysis_object``. Setting this to ``False`` can be useful if an ``analysis_object`` is performing a trajectory manipulation which is also useful for the - subsequent ``analysis_objects`` e.g. unwrapping of molecules. + subsequent ``analysis_instances`` e.g. unwrapping of molecules. """ # Ensure compatibility with API of version 0.15.0 - if not hasattr(self, "_analysis_objects"): - self._analysis_objects = (self,) + if not hasattr(self, "_analysis_instances"): + self._analysis_instances = (self,) logger.info("Choosing frames to analyze") # if verbose unchanged, use class default @@ -339,7 +349,7 @@ def run( logger.info("Starting preparation") - for analysis_object in self._analysis_objects: + for analysis_object in self._analysis_instances: analysis_object._setup_frames( analysis_object._trajectory, start=start, @@ -350,21 +360,26 @@ def run( analysis_object._prepare() logger.info( - f"Starting analysis loop over {self._analysis_objects[0].n_frames} " + f"Starting analysis loop over {self._analysis_instances[0].n_frames} " "trajectory frames." ) for i, ts in enumerate( - ProgressBar(self._analysis_objects[0]._sliced_trajectory, verbose=verbose) + ProgressBar(self._analysis_instances[0]._sliced_trajectory, verbose=verbose) ): if reset_timestep: ts_original = ts.copy() - for analysis_object in self._analysis_objects: + for analysis_object in self._analysis_instances: + # Set attributes before calling `_single_frame()`. Setting + # these attributes explicitly is mandatory so that each + # instance can access the information of the current timestep. analysis_object._frame_index = i analysis_object._ts = ts analysis_object.frames[i] = ts.frame analysis_object.times[i] = ts.time + + # Call the actual analysis of each instance. analysis_object._single_frame() if reset_timestep: @@ -372,7 +387,7 @@ def run( logger.info("Finishing up") - for analysis_object in self._analysis_objects: + for analysis_object in self._analysis_instances: analysis_object._conclude() return self diff --git a/testsuite/MDAnalysisTests/analysis/test_base.py b/testsuite/MDAnalysisTests/analysis/test_base.py index b17fb616d1..53b0bf8344 100644 --- a/testsuite/MDAnalysisTests/analysis/test_base.py +++ b/testsuite/MDAnalysisTests/analysis/test_base.py @@ -196,7 +196,7 @@ def _single_frame(self): def test_inconsistent_trajectory(self, universe): v = mda.Universe(TPR, XTC) - with pytest.raises(ValueError, match="`analysis_objects` do not have the same"): + with pytest.raises(ValueError, match="`analysis_instances` do not have the"): base.AnalysisCollection( InterRDF(universe.atoms, universe.atoms), InterRDF(v.atoms, v.atoms) ) @@ -207,7 +207,7 @@ def __init__(self, trajectory): self._trajectory = trajectory # Create collection for common trajectory loop with inconsistent trajectory - with pytest.raises(AttributeError, match="not a child of `AnalysisBa"): + with pytest.raises(AttributeError, match="not a child of `AnalysisBase`"): base.AnalysisCollection(CustomAnalysis(universe.trajectory)) From 3ba9d23d1f34ac38c65f04df0dd1983ef2a253cf Mon Sep 17 00:00:00 2001 From: Philip Loche Date: Thu, 23 Mar 2023 16:27:41 +0100 Subject: [PATCH 5/8] Remove reset_timestep option and typos --- package/CHANGELOG | 2 +- package/MDAnalysis/analysis/base.py | 29 +++++-------------- .../MDAnalysisTests/analysis/test_base.py | 12 +++----- 3 files changed, 13 insertions(+), 30 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 89b5984e10..ce1c91826c 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -34,7 +34,7 @@ Fixes (Issue #3336) Enhancements - * Add an `AnalayisCollection` class to perform multiple analysis on the same + * Add an `AnalaysisCollection` class to perform multiple analysis on the same trajectory (#3569, PR #4017). * Added AtomGroup TopologyAttr to calculate gyration moments (Issue #3904, PR #3905) diff --git a/package/MDAnalysis/analysis/base.py b/package/MDAnalysis/analysis/base.py index 35b6c2b223..b3ef360de8 100644 --- a/package/MDAnalysis/analysis/base.py +++ b/package/MDAnalysis/analysis/base.py @@ -224,7 +224,7 @@ def __setstate__(self, state): self.data = state -class AnalysisCollection(object): +class AnalysisCollection: """ Class for running a collection of analysis classes on a single trajectory. @@ -238,11 +238,7 @@ class AnalysisCollection(object): By default, it is ensured that all analysis instances use the *same original* timestep and not an altered one from a previous analysis - object. This behavior can be changed with the `reset_timestep` parameter - of the :meth:`MDAnalysis.analysis.base.AnalysisCollection.run` method. - Changing the default behaviour of `reset_timestep` might be useful to - avoid that subsequent analysis instances have to perform the same - transformation on the timestep. + object. Parameters ---------- @@ -313,7 +309,6 @@ def run( step=None, frames=None, verbose=None, - reset_timestep=True, ): """Perform the calculation @@ -332,11 +327,6 @@ def run( non-default value will raise a :exc:`ValueError`. verbose : bool, optional Turn on verbosity - reset_timestep : bool, optional - Reset the timestep object after for each ``analysis_object``. - Setting this to ``False`` can be useful if an ``analysis_object`` - is performing a trajectory manipulation which is also useful for the - subsequent ``analysis_instances`` e.g. unwrapping of molecules. """ # Ensure compatibility with API of version 0.15.0 @@ -367,8 +357,7 @@ def run( for i, ts in enumerate( ProgressBar(self._analysis_instances[0]._sliced_trajectory, verbose=verbose) ): - if reset_timestep: - ts_original = ts.copy() + ts_original = ts.copy() for analysis_object in self._analysis_instances: # Set attributes before calling `_single_frame()`. Setting @@ -382,8 +371,7 @@ def run( # Call the actual analysis of each instance. analysis_object._single_frame() - if reset_timestep: - ts = ts_original + ts = ts_original logger.info("Finishing up") @@ -489,7 +477,7 @@ def __init__(self, trajectory, verbose=False, **kwargs): self._trajectory = trajectory self._verbose = verbose self.results = Results() - super(AnalysisBase, self).__init__(self) + super().__init__(self) def _setup_frames(self, trajectory, start=None, stop=None, step=None, frames=None): @@ -594,7 +582,7 @@ def run(self, start=None, stop=None, step=None, frames=None, frame indices in the `frames` keyword argument. """ - return super(AnalysisBase, self).run( + return super().run( start=start, stop=stop, step=step, frames=frames, verbose=verbose ) @@ -671,7 +659,7 @@ def __init__(self, function, trajectory=None, *args, **kwargs): self.kwargs = kwargs - super(AnalysisFromFunction, self).__init__(trajectory) + super().__init__(trajectory) def _prepare(self): self.results.timeseries = [] @@ -739,8 +727,7 @@ def RotationMatrix(mobile, ref): class WrapperClass(AnalysisFromFunction): def __init__(self, trajectory=None, *args, **kwargs): - super(WrapperClass, self).__init__(function, trajectory, - *args, **kwargs) + super().__init__(function, trajectory, *args, **kwargs) return WrapperClass diff --git a/testsuite/MDAnalysisTests/analysis/test_base.py b/testsuite/MDAnalysisTests/analysis/test_base.py index 53b0bf8344..d0364e6676 100644 --- a/testsuite/MDAnalysisTests/analysis/test_base.py +++ b/testsuite/MDAnalysisTests/analysis/test_base.py @@ -166,8 +166,8 @@ def test_run(self, universe): assert rdf_OO.results is not None assert rdf_OH.results is not None - @pytest.mark.parametrize("reset_timestep", [True, False]) - def test_trajectory_manipulation(self, universe, reset_timestep): + def test_trajectory_manipulation(self, universe): + """Test that the timestep is the same for each analysis class.""" class CustomAnalysis(base.AnalysisBase): """Custom class that is shifting positions in every step by 10.""" @@ -185,13 +185,9 @@ def _single_frame(self): ana_2 = CustomAnalysis(universe.trajectory) collection = base.AnalysisCollection(ana_1, ana_2) + collection.run(frames=[0]) - collection.run(frames=[0], reset_timestep=reset_timestep) - - if reset_timestep: - assert ana_2.ref_pos == ana_1.ref_pos - else: - assert_allclose(ana_2.ref_pos, ana_1.ref_pos + 10) + assert ana_2.ref_pos == ana_1.ref_pos def test_inconsistent_trajectory(self, universe): v = mda.Universe(TPR, XTC) From 9ae6a51965e75e05d3fddd5e473d30100fb39f00 Mon Sep 17 00:00:00 2001 From: Philip Loche Date: Wed, 5 Jun 2024 17:28:53 +0200 Subject: [PATCH 6/8] move run into standalone private function --- package/MDAnalysis/analysis/base.py | 362 +++++++++--------- .../MDAnalysisTests/analysis/test_base.py | 14 +- 2 files changed, 193 insertions(+), 183 deletions(-) diff --git a/package/MDAnalysis/analysis/base.py b/package/MDAnalysis/analysis/base.py index 77b473f96c..29b0ab739e 100644 --- a/package/MDAnalysis/analysis/base.py +++ b/package/MDAnalysis/analysis/base.py @@ -125,6 +125,7 @@ tools if only the single-frame analysis function needs to be written. """ +from typing import Optional, Self, Tuple from collections import UserDict import inspect import logging @@ -224,164 +225,71 @@ def __setstate__(self, state): self.data = state -class AnalysisCollection: - """ - Class for running a collection of analysis classes on a single trajectory. - - Running a collection of analyses with ``AnalysisCollection`` can result in - a speedup compared to running the individual analyses since the trajectory - loop ins only performed once. - - The class assumes that each analysis is a child of - :class:`MDAnalysis.analysis.base.AnalysisBase`. Additionally, the - trajectory of all `analysis_instances` must be the same. - - By default, it is ensured that all analysis instances use the - *same original* timestep and not an altered one from a previous analysis - object. - - Parameters - ---------- - *analysis_instances : tuple - List of analysis classes to run on the same trajectory. - - Raises - ------ - AttributeError - If all the provided ``analysis_instances`` do not work on the same - trajectory. - AttributeError - If an ``analysis_object`` is not a child of - :class:`MDAnalysis.analysis.base.AnalysisBase`. - - Example - ------- - .. code-block:: python - - import MDAnalysis as mda - from MDAnalysis.analysis.rdf import InterRDF - from MDAnalysis.analysis.base import AnalysisCollection - from MDAnalysisTests.datafiles import TPR, XTC - - u = mda.Universe(TPR, XTC) - - # Select atoms - ag_O = u.select_atoms("name O") - ag_H = u.select_atoms("name H") - - # Create individual analysis instances - rdf_OO = InterRDF(ag_O, ag_O) - rdf_OH = InterRDF(ag_H, ag_H) - - # Create a collection for common trajectory - collection = AnalysisCollection(rdf_OO, rdf_OH) - - # Run the collected analysis - collection.run(start=0, stop=100, step=10) - - # Results are stored in the individual instances - print(rdf_OO.results) - print(rdf_OH.results) - - - .. versionadded:: 2.5.0 - - """ - - def __init__(self, *analysis_instances): - for analysis_object in analysis_instances: - if analysis_instances[0]._trajectory != analysis_object._trajectory: - raise ValueError( - "`analysis_instances` do not have the same trajectory." - ) - if not isinstance(analysis_object, AnalysisBase): - raise AttributeError( - f"Analysis object {analysis_object} is " - "not a child of `AnalysisBase`." - ) - - self._analysis_instances = analysis_instances - - def run( - self, - start=None, - stop=None, - step=None, - frames=None, - verbose=None, - ): - """Perform the calculation - - Parameters - ---------- - start : int, optional - start frame of analysis - stop : int, optional - stop frame of analysis - step : int, optional - number of frames to skip between each analysed frame - frames : array_like, optional - array of integers or booleans to slice trajectory; `frames` can - only be used *instead* of `start`, `stop`, and `step`. Setting - *both* `frames` and at least one of `start`, `stop`, `step` to a - non-default value will raise a :exc:`ValueError`. - verbose : bool, optional - Turn on verbosity - """ - - # Ensure compatibility with API of version 0.15.0 - if not hasattr(self, "_analysis_instances"): - self._analysis_instances = (self,) - - logger.info("Choosing frames to analyze") - # if verbose unchanged, use class default - verbose = getattr(self, "_verbose", False) if verbose is None else verbose - - logger.info("Starting preparation") - - for analysis_object in self._analysis_instances: - analysis_object._setup_frames( - analysis_object._trajectory, - start=start, - stop=stop, - step=step, - frames=frames, - ) - analysis_object._prepare() - - logger.info( - f"Starting analysis loop over {self._analysis_instances[0].n_frames} " - "trajectory frames." +def _run( + analysis_instances: Tuple["AnalysisBase", ...], + start: Optional[int] = None, + stop: Optional[int] = None, + step: Optional[int] = None, + frames: Optional[int] = None, + verbose: Optional[bool] = None, + progressbar_kwargs: Optional[dict] = None, +) -> None: + """Implementation of common run method.""" + + # if verbose unchanged, use default of first analysis_instance + verbose = getattr(analysis_instances[0], '_verbose', + False) if verbose is None else verbose + + logger.info("Choosing frames to analyze") + for analysis_object in analysis_instances: + analysis_object._setup_frames( + analysis_object._trajectory, + start=start, + stop=stop, + step=step, + frames=frames, ) - for i, ts in enumerate( - ProgressBar(self._analysis_instances[0]._sliced_trajectory, verbose=verbose) - ): - ts_original = ts.copy() - - for analysis_object in self._analysis_instances: - # Set attributes before calling `_single_frame()`. Setting - # these attributes explicitly is mandatory so that each - # instance can access the information of the current timestep. - analysis_object._frame_index = i - analysis_object._ts = ts - analysis_object.frames[i] = ts.frame - analysis_object.times[i] = ts.time - - # Call the actual analysis of each instance. - analysis_object._single_frame() + logger.info("Starting preparation") + for analysis_object in analysis_instances: + analysis_object._prepare() + + if progressbar_kwargs is None: + progressbar_kwargs = {} + + logger.info( + "Starting analysis loop over " + f"{analysis_instances[0].n_frames} trajectory frames" + ) + for i, ts in enumerate( + ProgressBar( + analysis_instances[0]._sliced_trajectory, + verbose=verbose, + **progressbar_kwargs, + ) + ): + ts_original = ts.copy() - ts = ts_original + for analysis_object in analysis_instances: + # Set attributes before calling `_single_frame()`. Setting + # these attributes explicitly is mandatory so that each + # instance can access the information of the current timestep. + analysis_object._frame_index = i + analysis_object._ts = ts + analysis_object.frames[i] = ts.frame + analysis_object.times[i] = ts.time - logger.info("Finishing up") + # Call the actual analysis of each instance. + analysis_object._single_frame() - for analysis_object in self._analysis_instances: - analysis_object._conclude() + ts = ts_original - return self + logger.info("Finishing up") + for analysis_object in analysis_instances: + analysis_object._conclude() -class AnalysisBase(AnalysisCollection): +class AnalysisBase: r"""Base class for defining multi-frame analysis The class is designed as a template for creating multi-frame analyses. @@ -477,7 +385,6 @@ def __init__(self, trajectory, verbose=False, **kwargs): self._trajectory = trajectory self._verbose = verbose self.results = Results() - super().__init__(self) def _setup_frames(self, trajectory, start=None, stop=None, step=None, frames=None): @@ -553,8 +460,13 @@ def _conclude(self): """ pass # pylint: disable=unnecessary-pass - def run(self, start=None, stop=None, step=None, frames=None, - verbose=None, *, progressbar_kwargs=None): + def run(self, + start: Optional[int] = None, + stop: Optional[int] = None, + step: Optional[int] = None, + frames: Optional[int] = None, + verbose: Optional[bool] = None, + progressbar_kwargs: Optional[dict] = None) -> Self: """Perform the calculation Parameters @@ -590,34 +502,128 @@ def run(self, start=None, stop=None, step=None, frames=None, Add `progressbar_kwargs` parameter, allowing to modify description, position etc of tqdm progressbars """ - logger.info("Choosing frames to analyze") - # if verbose unchanged, use class default - verbose = getattr(self, '_verbose', - False) if verbose is None else verbose - - self._setup_frames(self._trajectory, start=start, stop=stop, - step=step, frames=frames) - logger.info("Starting preparation") - self._prepare() - logger.info("Starting analysis loop over %d trajectory frames", - self.n_frames) - if progressbar_kwargs is None: - progressbar_kwargs = {} - - for i, ts in enumerate(ProgressBar( - self._sliced_trajectory, - verbose=verbose, - **progressbar_kwargs)): - self._frame_index = i - self._ts = ts - self.frames[i] = ts.frame - self.times[i] = ts.time - self._single_frame() - logger.info("Finishing up") - self._conclude() + _run(analysis_instances=(self,), + start=start, + stop=stop, + step=step, + frames=frames, + verbose=verbose, + progressbar_kwargs=progressbar_kwargs) + return self +class AnalysisCollection: + """ + Class for running a collection of analysis classes on a single trajectory. + + Running a collection of analyses with ``AnalysisCollection`` can result in + a speedup compared to running the individual analyses since the trajectory + loop ins only performed once. + + The class assumes that each analysis is a child of + :class:`MDAnalysis.analysis.base.AnalysisBase`. Additionally, the + trajectory of all `analysis_instances` must be the same. + + By default, it is ensured that all analysis instances use the + *same original* timestep and not an altered one from a previous analysis + object. + + Parameters + ---------- + *analysis_instances : tuple + List of analysis classes to run on the same trajectory. + + Raises + ------ + AttributeError + If all the provided ``analysis_instances`` do not work on the same + trajectory. + AttributeError + If an ``analysis_object`` is not a child of + :class:`MDAnalysis.analysis.base.AnalysisBase`. + + Example + ------- + .. code-block:: python + + import MDAnalysis as mda + from MDAnalysis.analysis.rdf import InterRDF + from MDAnalysis.analysis.base import AnalysisCollection + from MDAnalysisTests.datafiles import TPR, XTC + + u = mda.Universe(TPR, XTC) + + # Select atoms + ag_O = u.select_atoms("name O") + ag_H = u.select_atoms("name H") + + # Create individual analysis instances + rdf_OO = InterRDF(ag_O, ag_O) + rdf_OH = InterRDF(ag_H, ag_H) + + # Create a collection for common trajectory + collection = AnalysisCollection(rdf_OO, rdf_OH) + + # Run the collected analysis + collection.run(start=0, stop=100, step=10) + + # Results are stored in the individual instances + print(rdf_OO.results) + print(rdf_OH.results) + + + .. versionadded:: 2.8.0 + + """ + + def __init__(self, *analysis_instances): + for analysis_object in analysis_instances: + if ( + analysis_instances[0]._trajectory + != analysis_object._trajectory + ): + raise ValueError("`analysis_instances` do not have the same " + "trajectory.") + if not isinstance(analysis_object, AnalysisBase): + raise AttributeError(f"Analysis object {analysis_object} is " + "not a child of `AnalysisBase`.") + + self._analysis_instances = analysis_instances + + def run(self, + start: Optional[int] = None, + stop: Optional[int] = None, + step: Optional[int] = None, + frames: Optional[int] = None, + verbose: Optional[bool] = None, + progressbar_kwargs: Optional[dict] = None) -> None: + """Perform the calculation + + Parameters + ---------- + start : int, optional + start frame of analysis + stop : int, optional + stop frame of analysis + step : int, optional + number of frames to skip between each analysed frame + frames : array_like, optional + array of integers or booleans to slice trajectory; `frames` can + only be used *instead* of `start`, `stop`, and `step`. Setting + *both* `frames` and at least one of `start`, `stop`, `step` to a + non-default value will raise a :exc:`ValueError`. + verbose : bool, optional + Turn on verbosity + """ + _run(analysis_instances=self._analysis_instances, + start=start, + stop=stop, + step=step, + frames=frames, + verbose=verbose, + progressbar_kwargs=progressbar_kwargs) + class AnalysisFromFunction(AnalysisBase): r"""Create an :class:`AnalysisBase` from a function working on AtomGroups diff --git a/testsuite/MDAnalysisTests/analysis/test_base.py b/testsuite/MDAnalysisTests/analysis/test_base.py index c2eb340fc9..b0708c5528 100644 --- a/testsuite/MDAnalysisTests/analysis/test_base.py +++ b/testsuite/MDAnalysisTests/analysis/test_base.py @@ -192,9 +192,11 @@ def _single_frame(self): def test_inconsistent_trajectory(self, universe): v = mda.Universe(TPR, XTC) - with pytest.raises(ValueError, match="`analysis_instances` do not have the"): + match="`analysis_instances` do not have the same trajectory." + with pytest.raises(ValueError, match=match): base.AnalysisCollection( - InterRDF(universe.atoms, universe.atoms), InterRDF(v.atoms, v.atoms) + InterRDF( + universe.atoms, universe.atoms), InterRDF(v.atoms, v.atoms) ) def test_no_base_child(self, universe): @@ -202,8 +204,9 @@ class CustomAnalysis: def __init__(self, trajectory): self._trajectory = trajectory - # Create collection for common trajectory loop with inconsistent trajectory - with pytest.raises(AttributeError, match="not a child of `AnalysisBase`"): + match="not a child of `AnalysisBase`" + # collection for common trajectory loop with inconsistent trajectory + with pytest.raises(AttributeError, match=match): base.AnalysisCollection(CustomAnalysis(universe.trajectory)) @@ -312,7 +315,8 @@ def test_frames_times(u_xtc): assert an.n_frames == len(frames) assert_equal(an.found_frames, frames) assert_equal(an.frames, frames, err_msg=FRAMES_ERR) - assert_allclose(an.times, frames*100, rtol=0, atol=1.5e-4, err_msg=TIMES_ERR) + assert_allclose( + an.times, frames * 100, rtol=0, atol=1.5e-4, err_msg=TIMES_ERR) def test_verbose(u): From 34a9d7307a708f514916f6d04d100bef8b86a597 Mon Sep 17 00:00:00 2001 From: Philip Loche Date: Wed, 5 Jun 2024 18:07:50 +0200 Subject: [PATCH 7/8] remove Self typehint --- package/MDAnalysis/analysis/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package/MDAnalysis/analysis/base.py b/package/MDAnalysis/analysis/base.py index 29b0ab739e..e7d57a9ddf 100644 --- a/package/MDAnalysis/analysis/base.py +++ b/package/MDAnalysis/analysis/base.py @@ -125,7 +125,7 @@ tools if only the single-frame analysis function needs to be written. """ -from typing import Optional, Self, Tuple +from typing import Optional, Tuple from collections import UserDict import inspect import logging @@ -466,7 +466,7 @@ def run(self, step: Optional[int] = None, frames: Optional[int] = None, verbose: Optional[bool] = None, - progressbar_kwargs: Optional[dict] = None) -> Self: + progressbar_kwargs: Optional[dict] = None) -> "AnalysisBase": """Perform the calculation Parameters From e762e6aef7dbd042cedfd075e4da3a9c5b680fcd Mon Sep 17 00:00:00 2001 From: Philip Loche Date: Wed, 5 Jun 2024 18:09:49 +0200 Subject: [PATCH 8/8] lint --- package/MDAnalysis/analysis/base.py | 2 +- testsuite/MDAnalysisTests/analysis/test_base.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package/MDAnalysis/analysis/base.py b/package/MDAnalysis/analysis/base.py index e7d57a9ddf..dfc99c6de6 100644 --- a/package/MDAnalysis/analysis/base.py +++ b/package/MDAnalysis/analysis/base.py @@ -584,7 +584,7 @@ def __init__(self, *analysis_instances): != analysis_object._trajectory ): raise ValueError("`analysis_instances` do not have the same " - "trajectory.") + "trajectory.") if not isinstance(analysis_object, AnalysisBase): raise AttributeError(f"Analysis object {analysis_object} is " "not a child of `AnalysisBase`.") diff --git a/testsuite/MDAnalysisTests/analysis/test_base.py b/testsuite/MDAnalysisTests/analysis/test_base.py index b0708c5528..5ade7d9069 100644 --- a/testsuite/MDAnalysisTests/analysis/test_base.py +++ b/testsuite/MDAnalysisTests/analysis/test_base.py @@ -192,7 +192,7 @@ def _single_frame(self): def test_inconsistent_trajectory(self, universe): v = mda.Universe(TPR, XTC) - match="`analysis_instances` do not have the same trajectory." + match = "`analysis_instances` do not have the same trajectory." with pytest.raises(ValueError, match=match): base.AnalysisCollection( InterRDF( @@ -204,7 +204,7 @@ class CustomAnalysis: def __init__(self, trajectory): self._trajectory = trajectory - match="not a child of `AnalysisBase`" + match = "not a child of `AnalysisBase`" # collection for common trajectory loop with inconsistent trajectory with pytest.raises(AttributeError, match=match): base.AnalysisCollection(CustomAnalysis(universe.trajectory))