Skip to content

Replace deprecated assert_almost_equal with assert_allclose in GNM tests#5290

Closed
SRASHTI2004 wants to merge 1 commit intoMDAnalysis:developfrom
SRASHTI2004:fix-issues
Closed

Replace deprecated assert_almost_equal with assert_allclose in GNM tests#5290
SRASHTI2004 wants to merge 1 commit intoMDAnalysis:developfrom
SRASHTI2004:fix-issues

Conversation

@SRASHTI2004
Copy link

@SRASHTI2004 SRASHTI2004 commented Mar 8, 2026

Fixes #3743

Changes made in this Pull Request:

  • Replaced deprecated numpy.testing.assert_almost_equal with np.testing.assert_allclose in GNM tests.
  • Added absolute tolerance (atol=1e-4) to ensure numerical comparisons remain stable across platforms and NumPy versions.
  • Removed the deprecated import of assert_almost_equal.

LLM / AI generated code disclosure

LLMs or other AI-powered tools (beyond simple IDE use cases) were used in this contribution: no

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--5290.org.readthedocs.build/en/5290/

@codecov
Copy link

codecov bot commented Mar 8, 2026

Codecov Report

❌ Patch coverage is 94.14365% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.84%. Comparing base (9988556) to head (7c7aa79).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
package/MDAnalysis/coordinates/TRZ.py 89.69% 9 Missing and 1 partial ⚠️
package/MDAnalysis/visualization/streamlines_3D.py 64.70% 6 Missing ⚠️
package/MDAnalysis/analysis/align.py 50.00% 4 Missing ⚠️
package/MDAnalysis/coordinates/base.py 93.44% 4 Missing ⚠️
package/MDAnalysis/lib/transformations.py 42.85% 4 Missing ⚠️
...ckage/MDAnalysis/analysis/encore/confdistmatrix.py 62.50% 2 Missing and 1 partial ⚠️
package/MDAnalysis/analysis/encore/utils.py 33.33% 1 Missing and 1 partial ⚠️
package/MDAnalysis/topology/tpr/utils.py 92.59% 1 Missing and 1 partial ⚠️
package/MDAnalysis/analysis/bat.py 85.71% 1 Missing ⚠️
...e/MDAnalysis/analysis/encore/clustering/cluster.py 75.00% 0 Missing and 1 partial ⚠️
... and 16 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5290      +/-   ##
===========================================
+ Coverage    93.83%   93.84%   +0.01%     
===========================================
  Files          182      182              
  Lines        22501    22501              
  Branches      3195     3195              
===========================================
+ Hits         21113    21116       +3     
- Misses         925      928       +3     
+ Partials       463      457       -6     

☔ 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.

import MDAnalysis.analysis.gnm
import numpy as np
import pytest
from numpy.testing import assert_almost_equal
Copy link
Member

Choose a reason for hiding this comment

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

Given we previously did from numpy.testing import assert_almost_equal, I suggest you keep that pattern and import assert_allclose as well.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion!
I’ll keep the same import pattern and add assert_allclose alongside it to maintain consistency with the existing style. I’ll push the update shortly.

Copy link
Member

Choose a reason for hiding this comment

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

Don't know if we should be adding these - at the very least please add descriptions about what is being ignored & why like the rest of the file.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing that out!
I'll add proper descriptions explaining what is being ignored and why, to match the style used in the rest of the .gitignore file. I'll update the PR shortly.

Copy link
Contributor

@jeremyleung521 jeremyleung521 left a comment

Choose a reason for hiding this comment

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

There are 300 file changes (well, 299) in this PR, which I think are from a different linter that is not black~=24 (see docs and docs)? I would highly recommend fixing those and not including those changes here (esp. since the CI linter is throwing errors).

Again, it's making it really difficult tracking what you actually implemented, just like your previous PR.

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.

modernize testing code

3 participants