pin distopia<0.3.0 #4740

orbeckst · 2024-10-18T22:54:11Z

Fixes #4739

Changes made in this Pull Request:

temporarily restrict distopia in CI to >=0.2.0,<0.3.0 until MDAnalysis has caught up with distopia API changes
updated CHANGELOG
version check in code

PR Checklist

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

Developers certificate of origin

I certify that this contribution is covered by the LGPLv2.1+ license as defined in our LICENSE and adheres to the Developer Certificate of Origin.

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

codecov · 2024-10-18T23:03:15Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.58%. Comparing base (48e9e01) to head (6adbf74).
Report is 59 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4740      +/-   ##
===========================================
- Coverage    93.60%   93.58%   -0.02%     
===========================================
  Files          173      185      +12     
  Lines        21464    22536    +1072     
  Branches      2993     2994       +1     
===========================================
+ Hits         20091    21090     +999     
- Misses         929     1002      +73     
  Partials       444      444

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

orbeckst · 2024-10-18T23:29:48Z

package/CHANGELOG

@@ -78,6 +78,7 @@ Enhancements
   DOI 10.1021/acs.jpcb.7b11988. (Issue #2039, PR #4524)

 Changes
+ * only use distopia < 0.3.0 due to API changes (Issue #4739)


Not sure if we would normally make a CHANGELOG entry but in this case I would recommend to do so because there is not other part in the user-visible code base that knows about this requirement. If someone installs distopia by themselves, they will see failures.

Now they will (hopefully) a warning

../package/MDAnalysis/lib/_distopia.py:45 ~/MDAnalysis/mdanalysis/package/MDAnalysis/lib/_distopia.py:45: RuntimeWarning: Install 'distopia>=0.2.0,<0.3.0' to be used with this release of MDAnalysis. Your installed version of distopia >=0.3.0 will NOT be used. warnings.warn("Install 'distopia>=0.2.0,<0.3.0' to be used with this release "

.github/actions/setup-deps/action.yaml

- fix #4739 - temporarily restrict distopia to >=0.2.0,<0.3.0 until MDAnalysis has caught up with distopia API changes - updated CHANGELOG

hmacdope

LGTM @orbeckst, thanks for fixing.

hmacdope · 2024-10-18T23:32:04Z

Merge when ready

orbeckst · 2024-10-18T23:37:46Z

Let's hold on merging. I think @IAlibay also wanted a version check in the code and I agree as this will at least catch the issue with users installing distopia >= 0.3.0. Given the loose coupling between MDA and distopia, it seems necessary and you can then recycle it immediately for PR #4734 .

pep8speaks · 2024-10-19T01:05:51Z

Hello @orbeckst! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-10-19 01:11:18 UTC

- only enable HAS_DISTOPIA if the appropriate version of distopia is installed - issue RuntimeWarning if incorrect version present - added test

hmacdope · 2024-10-19T01:12:38Z

I will make a patch release for distopia that has __version__, sorry thats my bad.

orbeckst · 2024-10-19T01:13:48Z

I tested locally with distopia==0.2.0 and distopia=0.3.0 and the code behaves as it should: it fails for neither but warns for 0.3.0 and falls back to HAS_DISTOPIA=False.

orbeckst · 2024-10-19T01:15:05Z

package/MDAnalysis/lib/_distopia.py

+    # check for compatibility: currently needs to be >=0.2.0,<0.3.0 (issue
+    # #4740) No distopia.__version__ available so we have to do some probing.
+    needed_funcs = ['calc_bonds_no_box_float', 'calc_bonds_ortho_float']
+    has_distopia_020 = all([hasattr(distopia, func) for func in needed_funcs])


There's no distopia.__version__ attribute that I could check so I am explicitly probing for the functions we need.

@hmacdope is there a better way to get the version of the installed distopia package?

^ See comment above, I will make a patch release, I accidentally removed __version__

orbeckst · 2024-10-19T01:16:59Z

testsuite/MDAnalysisTests/lib/test_distances.py

@@ -788,9 +790,10 @@ def test_pbc_wrong_wassenaar_distance(self, backend):
        # expected.
        assert np.linalg.norm(point_a - point_b) != dist[0, 0]

-@pytest.mark.parametrize("box", 
+
+@pytest.mark.parametrize("box",


IAlibay

Thanks @orbeckst !

orbeckst · 2024-10-19T02:23:08Z

I'll merge with the clunky version probe to move things along and if needed we improve when there's a distopia 0.3.1.

orbeckst requested review from IAlibay and hmacdope October 18, 2024 23:27

orbeckst commented Oct 18, 2024

View reviewed changes

.github/actions/setup-deps/action.yaml Outdated Show resolved Hide resolved

pin distopia<0.3.0

e60f3d5

- fix #4739 - temporarily restrict distopia to >=0.2.0,<0.3.0 until MDAnalysis has caught up with distopia API changes - updated CHANGELOG

orbeckst force-pushed the pin-distopia branch from 43f83b4 to e60f3d5 Compare October 18, 2024 23:31

hmacdope approved these changes Oct 18, 2024

View reviewed changes

orbeckst force-pushed the pin-distopia branch from 4416672 to 575726b Compare October 19, 2024 01:10

version check distopia

6adbf74

- only enable HAS_DISTOPIA if the appropriate version of distopia is installed - issue RuntimeWarning if incorrect version present - added test

orbeckst force-pushed the pin-distopia branch from 575726b to 6adbf74 Compare October 19, 2024 01:11

orbeckst commented Oct 19, 2024

View reviewed changes

IAlibay approved these changes Oct 19, 2024

View reviewed changes

orbeckst self-assigned this Oct 19, 2024

orbeckst merged commit caa04ec into develop Oct 19, 2024
23 of 24 checks passed

orbeckst deleted the pin-distopia branch October 19, 2024 02:24

orbeckst mentioned this pull request Oct 19, 2024

Deprecate encore #4737

Merged

5 tasks

IAlibay mentioned this pull request Oct 19, 2024

Distopia warning shows up even when you don't have distopia installed #4742

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pin distopia<0.3.0 #4740

pin distopia<0.3.0 #4740

orbeckst commented Oct 18, 2024 •

edited

Loading

codecov bot commented Oct 18, 2024 •

edited

Loading

orbeckst Oct 18, 2024

orbeckst Oct 19, 2024

hmacdope left a comment

hmacdope commented Oct 18, 2024

orbeckst commented Oct 18, 2024

pep8speaks commented Oct 19, 2024 •

edited

Loading

hmacdope commented Oct 19, 2024

orbeckst commented Oct 19, 2024

orbeckst Oct 19, 2024

hmacdope Oct 19, 2024 •

edited

Loading

orbeckst Oct 19, 2024

IAlibay left a comment

orbeckst commented Oct 19, 2024

pin distopia<0.3.0 #4740

pin distopia<0.3.0 #4740

Conversation

orbeckst commented Oct 18, 2024 • edited Loading

PR Checklist

Developers certificate of origin

codecov bot commented Oct 18, 2024 • edited Loading

Codecov Report

orbeckst Oct 18, 2024

Choose a reason for hiding this comment

orbeckst Oct 19, 2024

Choose a reason for hiding this comment

hmacdope left a comment

Choose a reason for hiding this comment

hmacdope commented Oct 18, 2024

orbeckst commented Oct 18, 2024

pep8speaks commented Oct 19, 2024 • edited Loading

Comment last updated at 2024-10-19 01:11:18 UTC

hmacdope commented Oct 19, 2024

orbeckst commented Oct 19, 2024

orbeckst Oct 19, 2024

Choose a reason for hiding this comment

hmacdope Oct 19, 2024 • edited Loading

Choose a reason for hiding this comment

orbeckst Oct 19, 2024

Choose a reason for hiding this comment

IAlibay left a comment

Choose a reason for hiding this comment

orbeckst commented Oct 19, 2024

orbeckst commented Oct 18, 2024 •

edited

Loading

codecov bot commented Oct 18, 2024 •

edited

Loading

pep8speaks commented Oct 19, 2024 •

edited

Loading

hmacdope Oct 19, 2024 •

edited

Loading