Changes holetraj start/stop/step keywords (Issue #2513) (#2514)

* Fixes #2513
* contributes towards #2443
* Removes support for start/stop/step in :meth:HOLEtraj.__init__.
* Adds a check_slice_indices to the values passed to :meth:HOLEtraj.run.
* changes holetraj start/stop/step keywords
* update CHANGELOG

package/CHANGELOG

@@ -84,6 +84,10 @@ Enhancements
   * Improve the distance search in water bridge analysis with capped_distance (PR #2480)
 
 Changes
+  * Removes support for setting `start`/`stop`/`step` trajecotry slicing
+    keywords on :class:`HOLEtraj` construction. Also removes undocumented
+    support for passing :class:`HOLE` parameters to :meth:`HOLEtraj.run`
+    (Issue #2513). 
   * Removes the `nproc` keyword from
     :class:`Waterdynamics.HydrogenBondLifetimes` as the multiprocessing functionality
     did not work in some cases(Issues #2511).

package/MDAnalysis/analysis/hole.py

@@ -1172,9 +1172,9 @@ class HOLEtraj(BaseHOLE):
     universe and feed it to HOLE. It sequentially creates a temporary PDB for
     each frame and runs HOLE on the frame.
 
-    The trajectory can be sliced with the `start`, `stop`, and `step`
-    keywords. (:program:`hole` is not fast so slicing a trajectory is
-    recommended.)
+    The trajectory can be sliced by passing the `start`, `stop`, and `step`
+    keywords to :meth:`HOLEtraj.run`. (:program:`hole` is not fast so slicing a
+    trajectory is recommended.)
 
     Frames of the trajectory can be associated with order parameters (e.g.,
     RMSD) in order to group the HOLE profiles by order parameter (see the
@@ -1195,10 +1195,6 @@ def __init__(self, universe, **kwargs):
         orderparameters : array_like or string, optional
              Sequence or text file containing order parameters (float
              numbers) corresponding to the frames in the trajectory.
-        start, stop, step : int, optional
-             slice the trajectory as
-             ``universe.trajectory[start:stop:step]``. The default is ``None``
-             so that the whole trajectory is analyzed
         selection : string, optional
              selection string for
              :meth:`~MDAnalysis.core.universe.Universe.select_atoms` to select
@@ -1218,16 +1214,16 @@ def __init__(self, universe, **kwargs):
              All other keywords are passed on to :class:`HOLE` (see there for
              description).
 
+
+        .. versionchanged:: 1.0.0
+           Support for the `start`, `stop`, and `step` keywords has been
+           removed. These should instead be passed to :meth:`HOLEtraj.run`.
         """
 
         self.universe = universe
         self.selection = kwargs.pop("selection", "protein")
         self.orderparametersfile = kwargs.pop("orderparameters", None)
 
-        self.start = kwargs.pop('start', None)
-        self.stop = kwargs.pop('stop', None)
-        self.step = kwargs.pop('step', None)
-
         self.cpoint = kwargs.pop('cpoint', None)
         if self.cpoint is True:
             self.cpoint = self.guess_cpoint(selection=self.selection)
@@ -1275,17 +1271,29 @@ def _process_orderparameters(self, data):
             raise ValueError(errmsg)
         return q
 
-    def run(self, **kwargs):
+    def run(self, start=None, stop=None, step=None):
         """Run HOLE on the whole trajectory and collect profiles.
 
-        Keyword arguments `start`, `stop`, and `step` can be used to only
-        analyse part of the trajectory.
+        Parameters
+        ----------
+        start : int, optional
+            First frame of trajectory to analyse, Default: None (first frame)
+        stop : int, optional
+            Last frame of trajectory to analyse, Default: None (last frame)
+        step : int, optional
+            Step between frames to analyse, Default: None (use every frame)
+
+        .. versionchanged:: 1.0.0
+           Undocumented support for setting :class:`HOLE` parameters via
+           :meth:`HOLEtraj.run` has been removed. This should now exclusively
+           be done on :class:`HOLEtraj` construction.
         """
-        start = kwargs.pop('start', self.start)
-        stop = kwargs.pop('stop', self.stop)
-        step = kwargs.pop('step', self.step)
+        start, stop, step = self.universe.trajectory.check_slice_indices(
+            start,
+            stop,
+            step
+        )
         hole_kw = self.hole_kwargs.copy()
-        hole_kw.update(kwargs)
 
         profiles = OrderedDict()  # index by orderparameters: NOTE: can overwrite!
 

testsuite/MDAnalysisTests/analysis/test_hole.py

@@ -101,8 +101,8 @@ def universe(self):
     @pytest.fixture()
     def H(self, universe, tmpdir):
         with tmpdir.as_cwd():
-            H = HOLEtraj(universe, start=self.start, stop=self.stop, raseed=31415)
-            H.run()
+            H = HOLEtraj(universe, raseed=31415)
+            H.run(start=self.start, stop=self.stop)
         return H
 
     @pytest.fixture()