Skip to content

Fix #3743: Modernize deprecated test assertions in hydrogen bond tests#5400

Open
ShivansGupta25 wants to merge 2 commits into
MDAnalysis:developfrom
ShivansGupta25:fix-3743-modernize-tests
Open

Fix #3743: Modernize deprecated test assertions in hydrogen bond tests#5400
ShivansGupta25 wants to merge 2 commits into
MDAnalysis:developfrom
ShivansGupta25:fix-3743-modernize-tests

Conversation

@ShivansGupta25

@ShivansGupta25 ShivansGupta25 commented Jun 17, 2026

Copy link
Copy Markdown

Summary

Modernize test assertions in test_hydrogenbonds_analysis.py by replacing deprecated NumPy testing helpers with current testing practices.

Changes

  • Removed unused assert_almost_equal import.
  • Replaced assert_almost_equal assertions with more appropriate modern alternatives.
  • Replaced floating-point comparisons with numpy.testing.assert_allclose where applicable.
  • Preserved existing test behavior and tolerances.
  • Updated assertions to better match the type of values being tested.

Testing

pytest testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py -q

Result:

79 passed

AI Declaration

  • I used AI assistance while working on this PR.

AI was used for:

  • Understanding the issue requirements.
  • Identifying deprecated testing patterns.
  • Suggesting modern assertion alternatives.
  • Reviewing proposed test changes.

All suggested changes were manually reviewed, verified, and tested by me before submission.

@ShivansGupta25 ShivansGupta25 marked this pull request as ready for review June 17, 2026 16:38
@IAlibay

IAlibay commented Jun 18, 2026

Copy link
Copy Markdown
Member

@ShivansGupta25 Thanks for opening this PR. However, before we can review we ask you put back and answer the AI declaration which was the in the PR template.

@IAlibay IAlibay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking until relevant declarations are added back into the PR description.

@ShivansGupta25 ShivansGupta25 changed the title Replace assert_almost_equal with pytest.approx in hydrogen bond tests Fix #3743: Modernize deprecated test assertions in hydrogen bond tests Jun 18, 2026
@ShivansGupta25

Copy link
Copy Markdown
Author

@ShivansGupta25 Thanks for opening this PR. However, before we can review we ask you put back and answer the AI declaration which was the in the PR template.

I've restored and completed the AI declaration section. All changes were reviewed and tested locally. Thanks for pointing it out sir.

@ShivansGupta25 ShivansGupta25 requested a review from IAlibay June 18, 2026 13:40
@ShivansGupta25 ShivansGupta25 force-pushed the fix-3743-modernize-tests branch from 814a445 to cec6637 Compare June 18, 2026 13:47

counts = h.count_by_time()
assert_array_almost_equal(h.times, ref_times)
assert_allclose(h.times, ref_times)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

assert_allclose without args is actually stricter than the old assert_array_almost_equal. In a similar PR (#5156), the fix was to add atol=1e-6, rtol=0 to match the old tolerance. Might be worth doing the same here

Suggested change
assert_allclose(h.times, ref_times)
assert_allclose(h.times, ref_times, atol=1e-6, rtol=0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants