nojump.py raises a valueerror when applied not at first frame

[spyke7]

Fixes #4915

Changes made in this Pull Request:

• Added a check inside nojump.py under the _transform function that checks if the self.prev is None and ts.frame != 0
• it raises a ValueError
• Updated the test_nojump.py that checks for -1 and also midway

LLM / AI generated code disclosure

LLMs or other AI-powered tools (beyond simple IDE use cases) were used in this contribution: no
(not needed, I guess this was simple)

PR Checklist

• Issue raised/referenced?
• Tests updated/added?
• Documentation updated/added?
• package/CHANGELOG file updated?
• Is your name in package/AUTHORS? (If it is not, add it!)
• LLM/AI disclosure was updated.

Developers Certificate of Origin

I certify that I can submit this code contribution as described in the Developer Certificate of Origin, under the MDAnalysis LICENSE.

---

📚 Documentation preview 📚: https://mdanalysis--5201.org.readthedocs.build/en/5201/

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.72%. Comparing base (8ff4d09) to head (29e77f4).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #5201      +/-   ##
===========================================
- Coverage    92.72%   92.72%   -0.01%     
===========================================
  Files          180      180              
  Lines        22474    22475       +1     
  Branches      3189     3190       +1     
===========================================
  Hits         20840    20840              
- Misses        1176     1177       +1     
  Partials       458      458              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tylerjereddy]

I'm a bit confused by the _ = ... lines here and above. Why are they different in each case?

In my hands, I can just delete both of those lines, and the tests still pass, so that seems preferable:

--- a/testsuite/MDAnalysisTests/transformations/test_nojump.py
+++ b/testsuite/MDAnalysisTests/transformations/test_nojump.py
@@ -393,7 +393,6 @@ def test_nojump_fails_when_not_at_frame_0():
         ValueError, match="must be applied starting from frame 0"
     ):
         u.trajectory.add_transformations(NoJump())
-        _ = u.trajectory[0]
 
 
 def test_nojump_fails_midtrajectory():
@@ -407,4 +406,3 @@ def test_nojump_fails_midtrajectory():
         ValueError, match="must be applied starting from frame 0"
     ):
         u.trajectory.add_transformations(NoJump())
-        _ = u.trajectory.timeseries()

Then, after making that change, it should be trivial to condense this into a single parametrized test to reduce duplication.

[spyke7]

_ = u.trajectory[0] I thought that _transform() was called when you access a frame, so this was to trigger frame access, and _ = u.trajectory.timeseries() - to trigger iteration
So, I will remove it, the variables was indeed not necessary.

[BradyAJohnston]

You combine these two separate tests into a single parameterized test, passing in the two values -1 and 5 to test both cases rather than duplicating code.

[spyke7]

ok, that seems nice

Comments were addressed

[tylerjereddy]

I'll squash merge it -- it looks like Brady had to repeat my request for test parametrization, and the changelog entry could be a bit clearer (i.e., this behavior already errored out, it was just the error type and the content of the error message that was improved). I think the changelog entry also has a spurious/extra space in it, but doesn't seem like something we need to block on.

[spyke7]

Ok, then.
I will take care of the changelog entry from the next PR
Thanks for your guidance