Remove start/stop/step from AnalysisBase kwargs

[IAlibay]

Addresses #1463

Changes made in this Pull Request:

• Removed support for start, stop, and step keywords on object creation for :class:AnalysisBase and :class:AnalysisFromFunction
• Where appropriate docstring was changed. If the docstring indicated that start, stop, and step keywords could be passed to __init__ a versionchanged tag was added.

To do:

• Currently the tests for AnalysisFromFunction don't seem to test out the start/stop arguments very well, it's probably worth adding an extra test there.

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[IAlibay]

I think that should be everything for AnalysisBase, pinging @orbeckst here as the author of #1463

[codecov]

Codecov Report

Merging #2505 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2505   +/-   ##
========================================
  Coverage    90.47%   90.47%           
========================================
  Files          170      170           
  Lines        23091    23091           
  Branches      2980     2980           
========================================
  Hits         20891    20891           
  Misses        1596     1596           
  Partials       604      604

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b8bd9d...7b34474. Read the comment docs.

[orbeckst]

Already looking good but I found a few more — can you please deal with them, too? Thanks!

• Contacts.q1q2: https://github.com/IAlibay/mdanalysis/blob/4f6e3ede7e859fa621b2d41d2cac87e35af01137/package/MDAnalysis/analysis/contacts.py#L472
• HOLEtraj is not AnalysisBase but it already looks like it and it would be easy to remove start/stop/step https://github.com/IAlibay/mdanalysis/blob/4f6e3ede7e859fa621b2d41d2cac87e35af01137/package/MDAnalysis/analysis/hole.py#L1208 as you can just set them to defaults in run() at https://github.com/IAlibay/mdanalysis/blob/4f6e3ede7e859fa621b2d41d2cac87e35af01137/package/MDAnalysis/analysis/hole.py#L1294-L1296 (namely start, stop, step = None, None, None)
• in waterdynamics, the old hbonds.HydrogenBondAnalysis is used with start/stop constructor (https://github.com/IAlibay/mdanalysis/blob/4f6e3ede7e859fa621b2d41d2cac87e35af01137/package/MDAnalysis/analysis/waterdynamics.py#L644-L650): can you change this to call it from h.run() https://github.com/IAlibay/mdanalysis/blob/4f6e3ede7e859fa621b2d41d2cac87e35af01137/package/MDAnalysis/analysis/waterdynamics.py#L653 (for consistency, even if we don't touch hbonds)
• waterdynamics.SurvivalProbability also deprecated start/stop/step https://github.com/IAlibay/mdanalysis/blob/4f6e3ede7e859fa621b2d41d2cac87e35af01137/package/MDAnalysis/analysis/waterdynamics.py#L1248-L1260 — can you remove this code, too, please? (I also noted that here stop is inclusive, which is contrary to everwhere else (stop is exclusive) — this should changed before 1.0 but needs to be documented as an API-breaking change): either fix here in the start/stop/step PR or raise an issue if this looks more complicated an needs review from some waterdynamics authors.

[orbeckst]

@IAlibay only contacts.q1q2 is strictly part of this PR re: AnalysisBase. I would also be ok with only fixing q1q2 and then opening another issue/PR for harmonizing start/stop/step in non-standard methods with the AnalysisBase model. I leave it up to you and how much time you can spare. Thank you so much for moving the code towards 1.0 — really much appreciated!!

[IAlibay]

@orbeckst thanks for the review 😄

So going by #1463 (comment) my plan was to do all the :class:AnalysisBase changes in this PR and then raise a separate PR for the non-AnalysisBase stuff (I have already started working on the waterdynamics stuff on a separate branch).

Going by #1463 (comment) would you prefer it if I raised the non-AnalysisBase required changes as separate issues or is the dicussion here sufficient?

[IAlibay]

Aside from q1q2, I'll also add the waterdynamics hbonds call to this PR since this directly involves a :class:AnalysisBase method.

A note (to self) here: pytest didn't complain when running test_waterdynamics, and it should have. Extra tests will be needed here.

[orbeckst]

Going by #1463 (comment) would you prefer it if I raised the non-AnalysisBase required changes as separate issues or is the dicussion here sufficient?

Please raise a separate issue, then we can cleanly close this one, and have a new rallying point for new things that might rear their ugly heads. Thanks!

[IAlibay]

Unfortunately encountered #2511 whilst fixing this, it'll need to be fixed before this PR can be tied off.

[IAlibay]

With #2520 completed, this should be complete for :class:AnalysisBase

[orbeckst]

Many thanks @IAlibay !

This should be merged once CI is good.