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.

Sign up for GitHub

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 the versionchanged for frames #3710

Merged
merged 1 commit into from
Jun 9, 2022
Merged

fix the versionchanged for frames #3710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package/CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ The rules for this file:
* release numbers follow "Semantic Versioning" http://semver.org

------------------------------------------------------------------------------
??/??/?? IAlibay, PicoCentauri
??/??/?? IAlibay, PicoCentauri, orbeckst

* 2.3.0

Expand All @@ -23,6 +23,7 @@ Fixes
Enhancements
* Add `norm` parameter to InterRDF, InterRDF_s to normalize as rdf,
number density or do not normalize at all. (Issue #3687)
* Additional logger.info output when per-frame analysis starts (PR #3710)
Copy link
Member Author

Choose a reason for hiding this comment

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

It's minor but might actually be useful if one wants to see in the log when the per-frame loop starts and stops (that's one way to get timing information even if one only enabled logging).


Changes

Expand Down
74 changes: 47 additions & 27 deletions package/MDAnalysis/analysis/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,10 @@
class Results(UserDict):
r"""Container object for storing results.

``Results`` are dictionaries that provide two ways by which values can be
accessed: by dictionary key ``results["value_key"]`` or by object
attribute, ``results.value_key``. ``Results`` stores all results obtained
from an analysis after calling :func:`run()`.
:class:`Results` are dictionaries that provide two ways by which values
can be accessed: by dictionary key ``results["value_key"]`` or by object
attribute, ``results.value_key``. :class:`Results` stores all results
obtained from an analysis after calling :meth:`~AnalysisBase.run()`.

The implementation is similar to the :class:`sklearn.utils.Bunch`
class in `scikit-learn`_.
Expand Down Expand Up @@ -228,7 +228,7 @@ class AnalysisBase(object):
reader for iterating, and it offers to show a progress meter.
Computed results are stored inside the :attr:`results` attribute.

To define a new Analysis, `AnalysisBase` needs to be subclassed
To define a new Analysis, :class:`AnalysisBase` needs to be subclassed
and :meth:`_single_frame` must be defined. It is also possible to define
:meth:`_prepare` and :meth:`_conclude` for pre- and post-processing.
All results should be stored as attributes of the :class:`Results`
Expand Down Expand Up @@ -304,12 +304,12 @@ def _conclude(self):


.. versionchanged:: 1.0.0
Support for setting ``start``, ``stop``, and ``step`` has been
removed. These should now be directly passed to
:meth:`AnalysisBase.run`.
Support for setting `start`, `stop`, and `step` has been removed. These
Copy link
Member Author

Choose a reason for hiding this comment

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

use 79 character line width

should now be directly passed to :meth:`AnalysisBase.run`.

.. versionchanged:: 2.0.0
Added :attr:`results`

"""

def __init__(self, trajectory, verbose=False, **kwargs):
Expand All @@ -319,8 +319,7 @@ def __init__(self, trajectory, verbose=False, **kwargs):

def _setup_frames(self, trajectory, start=None, stop=None, step=None,
frames=None):
"""
Pass a Reader object and define the desired iteration pattern
"""Pass a Reader object and define the desired iteration pattern
through the trajectory

Parameters
Expand All @@ -334,15 +333,25 @@ 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
array of integers or booleans to slice trajectory; cannot be
combined with `start`, `stop`, `step`

.. versionadded:: 2.2.0

Raises
------
ValueError
orbeckst marked this conversation as resolved.
Show resolved Hide resolved
if *both* `frames` and at least one of `start`, `stop`, or `frames`
is provided (i.e., set to another value than ``None``)


.. versionchanged:: 1.0.0
Added .frames and .times arrays as attributes

.. versionchanged:: 2.2.0
Added ability to iterate through trajectory by passing a list of
frame indices
frame indices in the `frames` keyword argument

"""
self._trajectory = trajectory
if frames is not None:
Expand Down Expand Up @@ -393,14 +402,21 @@ 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
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

verbose : bool, optional
Turn on verbosity


.. versionchanged:: 2.1.0
Added ability to iterate through trajectory by passing a list of
frame indices
.. versionchanged:: 2.2.0
Added ability to analyze arbitrary frames by passing a list of
frame indices in the `frames` keyword argument.

"""
logger.info("Choosing frames to analyze")
# if verbose unchanged, use class default
Expand All @@ -411,6 +427,8 @@ def run(self, start=None, stop=None, step=None, frames=None,
step=step, frames=frames)
logger.info("Starting preparation")
self._prepare()
logger.info("Starting analysis loop over %d trajectory frames",
self.n_frames)
Comment on lines +430 to +431
Copy link
Member Author

Choose a reason for hiding this comment

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

We announce frame selection, preparation and conclusion but not the main analysis loop. Adding this logger call makes it visible in a log file and allows reasonable ad-hoc benchmarking because the number of frames analyzed is also printed: one can calculate ((time of logger call conclusions) - (time of call starting analysis))/n_frames.

for i, ts in enumerate(ProgressBar(
self._sliced_trajectory,
verbose=verbose)):
Expand All @@ -431,13 +449,13 @@ class AnalysisFromFunction(AnalysisBase):
----------
function : callable
function to evaluate at each frame
trajectory : mda.coordinates.Reader, optional
trajectory : MDAnalysis.coordinates.Reader, optional
trajectory to iterate over. If ``None`` the first AtomGroup found in
args and kwargs is used as a source for the trajectory.
*args : list
arguments for ``function``
arguments for `function`
**kwargs : dict
arguments for ``function`` and :class:`AnalysisBase`
arguments for `function` and :class:`AnalysisBase`

Attributes
----------
Expand All @@ -452,7 +470,7 @@ class AnalysisFromFunction(AnalysisBase):
Raises
------
ValueError
if ``function`` has the same ``kwargs`` as :class:`AnalysisBase`
if `function` has the same `kwargs` as :class:`AnalysisBase`
Copy link
Member Author

Choose a reason for hiding this comment

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

follow Style Guide and reference args and kwargs with single backticks


Example
-------
Expand All @@ -467,9 +485,9 @@ def rotation_matrix(mobile, ref):


.. versionchanged:: 1.0.0
Support for directly passing the ``start``, ``stop``, and ``step``
arguments has been removed. These should instead be passed
to :meth:`AnalysisFromFunction.run`.
Support for directly passing the `start`, `stop`, and `step` arguments
has been removed. These should instead be passed to
:meth:`AnalysisFromFunction.run`.

.. versionchanged:: 2.0.0
Former :attr:`results` are now stored as :attr:`results.timeseries`
Expand Down Expand Up @@ -533,7 +551,7 @@ def analysis_class(function):
Raises
------
ValueError
if ``function`` has the same ``kwargs`` as :class:`AnalysisBase`
if `function` has the same `kwargs` as :class:`AnalysisBase`

Examples
--------
Expand All @@ -559,7 +577,7 @@ def RotationMatrix(mobile, ref):


.. versionchanged:: 2.0.0
Former ``results`` are now stored as ``results.timeseries``
Former :attr:`results` are now stored as :attr:`results.timeseries`
"""

class WrapperClass(AnalysisFromFunction):
Expand All @@ -572,7 +590,8 @@ def __init__(self, trajectory=None, *args, **kwargs):

def _filter_baseanalysis_kwargs(function, kwargs):
"""
create two dictionaries with kwargs separated for function and AnalysisBase
Create two dictionaries with `kwargs` separated for `function` and
:class:`AnalysisBase`

Parameters
----------
Expand All @@ -591,7 +610,8 @@ def _filter_baseanalysis_kwargs(function, kwargs):
Raises
------
ValueError
if ``function`` has the same ``kwargs`` as :class:`AnalysisBase`
if `function` has the same `kwargs` as :class:`AnalysisBase`

"""
try:
# pylint: disable=deprecated-method
Expand Down