Skip to content

Standardize atomicdistances to use results#5347

Open
charity-g wants to merge 18 commits intoMDAnalysis:developfrom
charity-g:standardize-atomicdistances-to-use-results
Open

Standardize atomicdistances to use results#5347
charity-g wants to merge 18 commits intoMDAnalysis:developfrom
charity-g:standardize-atomicdistances-to-use-results

Conversation

@charity-g
Copy link
Copy Markdown

@charity-g charity-g commented Mar 28, 2026

Fixes #4819

Changes made in this Pull Request:

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

Copy link
Copy Markdown
Member

@BradyAJohnston BradyAJohnston left a comment

Choose a reason for hiding this comment

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

Some suggestions that should fix the issues with running tests.

The black formatter is also failing. You can run it like this:

uvx black~=24.0 package

@orbeckst
Copy link
Copy Markdown
Member

@charity-g based on discussion in #4822 (comment) we will continue with this PR. Have a look at @BradyAJohnston 's comments and feel free to take it out of draft mode.

Copy link
Copy Markdown
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

The first thing is to make the code (and tests) run, following Brady's comments. Then please address my inline comments. Focus this PR just on the API change. CHANGELOG and docs must be very clear.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.84%. Comparing base (3be9117) to head (8eeb972).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5347      +/-   ##
===========================================
- Coverage    93.84%   93.84%   -0.01%     
===========================================
  Files          182      182              
  Lines        22491    22493       +2     
  Branches      3198     3198              
===========================================
+ Hits         21107    21108       +1     
- Misses         922      923       +1     
  Partials       462      462              

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

@charity-g charity-g marked this pull request as ready for review April 11, 2026 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

analysis.atomicdistances.AtomicDistances does not use Results

6 participants