Ensure Timestep in ChainReader is updated for multiple trajectory files

[jaclark5]

Fixes #3657

Changes made in this Pull Request:

• If statement in transform added to ensure that the provided Timestep class is the same used in the Atom Group trajectory. This is important when multiple trajectory files are used, otherwise the first step of the next trajectory will not undergo a transform.

PR Checklist

• Tests?
• [NA] Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[codecov]

Codecov Report

Base: 94.38% // Head: 94.38% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (2affb60) compared to base (3712439).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3834   +/-   ##
========================================
  Coverage    94.38%   94.38%           
========================================
  Files          194      194           
  Lines        25242    25250    +8     
  Branches      3493     3497    +4     
========================================
+ Hits         23825    23833    +8     
  Misses        1368     1368           
  Partials        49       49           

Impacted Files	Coverage Δ
package/MDAnalysis/transformations/fit.py	100.00% <100.00%> (ø)
package/MDAnalysis/transformations/wrap.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jaclark5]

@tylerjereddy I cleared it on the original issue, but you'll see the edits to applicable transforms with tests have been merged.

[jaclark5]

The CodeCoverage stipulation is failing because of the most recent merge into the development branch related to RDKit... is there a work around for this? The code relevant to this pull request is fully covered.

[tylerjereddy]

I think you're good now as far as codecov checks are concerned. I think they can take a while to properly aggregate.

[richardjgowers]

I had a dig into this. I think this fix is probably fine, but I think there's a more fundamental issue with ChainReader not respecting the Reader base class, which makes lots of annoying bugs like this appear. I think a better fix (which I'll try and put together) is to put ChainReader under the boot of ProtoReader, rather than have it be "equal".

[jaclark5]

Ok sure, let me know when you decide to go with this fix or if I should delete it.

To find this solution I followed the order of operations in updating the timestep and this cuts through the chicken and the egg cycle. I didn't see another solution, but I'm a novice with MDAnalysis' structure.

[jaclark5]

@richardjgowers any thoughts on this? I think this is a glaring issue in MDAnalysis, although it seems that an analysis that relies on capped_distances won't be affected as long as a box is defined to correct it. However, if one were to attempt to make a movie like the "Centering a trajectory in the box" example for a case with multiple trajectories, they would not achieve a clean movie. The solution presented here resolves the issue simply for the time being.