Skip to content

mdakits are now optional deps#4953

Merged
IAlibay merged 9 commits intodevelopfrom
optional-mdakits
Mar 10, 2025
Merged

mdakits are now optional deps#4953
IAlibay merged 9 commits intodevelopfrom
optional-mdakits

Conversation

@IAlibay
Copy link
Copy Markdown
Member

@IAlibay IAlibay commented Mar 9, 2025

Fixes #4899

Changes made in this Pull Request:

  • All mdakits are now optional deps

PR Checklist

  • Issue raised/referenced?
  • Tests updated/added?
  • Documentation updated/added?
  • package/CHANGELOG file 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.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 9, 2025

Codecov Report

Attention: Patch coverage is 65.21739% with 8 lines in your changes missing coverage. Please review.

Project coverage is 93.62%. Comparing base (bd00f88) to head (c79729d).
Report is 26 commits behind head on develop.

Files with missing lines Patch % Lines
package/MDAnalysis/analysis/hole2/__init__.py 55.55% 4 Missing ⚠️
package/MDAnalysis/analysis/psa.py 71.42% 2 Missing ⚠️
package/MDAnalysis/analysis/waterdynamics.py 71.42% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4953      +/-   ##
===========================================
- Coverage    93.68%   93.62%   -0.06%     
===========================================
  Files          177      189      +12     
  Lines        21845    22923    +1078     
  Branches      3077     3077              
===========================================
+ Hits         20465    21462     +997     
- Misses         930     1011      +81     
  Partials       450      450              

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

@IAlibay IAlibay marked this pull request as ready for review March 9, 2025 19:58
"and will be removed in MDAnalysis version 3.0.0"
)
warnings.warn(wmsg, category=DeprecationWarning)
try:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure why these are reporting as not covered - the tests for both warnings seem to be passing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I manually tested everything locally (with and without the optional imports) so I'd say it's probably worth ignoring codecov here.

@IAlibay IAlibay mentioned this pull request Mar 9, 2025
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.

LGTM, thanks!

Minor spelling fix for CHANGELOG as suggestion.

Docs (on RTD) do not include detailed code anymore (?) but they do have the link to the MDAKit and the deprecation note and that's good enough for me.

Comment thread package/CHANGELOG Outdated
Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>
@IAlibay
Copy link
Copy Markdown
Member Author

IAlibay commented Mar 10, 2025

Docs (on RTD) do not include detailed code anymore (?)

Just checked the latest develop build and it looks like this has been the case for a while (I think I remember the two of us having that same conversation some time back and iirc we agreed that it would be easier to not duplicate docs & tests across two repos).

@IAlibay IAlibay merged commit 596cd7c into develop Mar 10, 2025
22 of 24 checks passed
@IAlibay IAlibay deleted the optional-mdakits branch March 10, 2025 02:04
@RMeli
Copy link
Copy Markdown
Member

RMeli commented Mar 10, 2025

+1 for not duplicating documentation and tests. A link to a single source of truth is definitely the best approach.

Abdulrahman-PROG pushed a commit to Abdulrahman-PROG/mdanalysis that referenced this pull request Apr 13, 2025
* mdakits are now optional deps
---------

Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>
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.

Make all mdakits imported & used in the core library optional and don't ship them on the conda-forge recipe with v2.9

3 participants