mdanalysis icon indicating copy to clipboard operation
mdanalysis copied to clipboard

replaced lib.transformations with transformations package

Open orbeckst opened this issue 9 months ago • 7 comments

Fixes #5061

Changes made in this Pull Request:

  • removed old lib.transformations code
  • use transformations package instead (which is the updated version of our old vendored code)
  • Makes transformations a core dependency.
  • moved our only addition (transformations.rotaxis() to mdamath.rotaxis()) and left a deprecated version in lib.transformations

PR Checklist

  • [x] Issue raised/referenced?
  • [x] Tests updated/added?
  • [x] Documentation updated/added?
  • [x] package/CHANGELOG file updated?
  • [x] Is your name in package/AUTHORS? (If it is not, add it!)

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

orbeckst avatar Jun 11 '25 01:06 orbeckst

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.11%. Comparing base (8b0be5f) to head (bd88561). Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5062      +/-   ##
===========================================
+ Coverage    93.62%   94.11%   +0.48%     
===========================================
  Files          177      177              
  Lines        22001    21331     -670     
  Branches      3114     3021      -93     
===========================================
- Hits         20599    20075     -524     
+ Misses         947      847     -100     
+ Partials       455      409      -46     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Jun 11 '25 06:06 codecov[bot]

If you both think that’s feasible then it may still be a positive trade off for us: reduce code burden inside MDA (and magically increase coverage by 0.5%) and do something good for the community.  Who would start the process of getting on the cf feedstock recipe?Am 6/12/25 um 03:57 schrieb Irfan Alibay @.***>: @IAlibay commented on this pull request.

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

  • transformations:
  • default: 'transformations'

I agree with @RMeli - the recipe seems rather simple so it might not be too much of a hassle for us to take it on.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

orbeckst avatar Jun 12 '25 14:06 orbeckst

Who would start the process of getting on the cf feedstock recipe?

I have opened an issue on the feedstock, let's see what happens.

IAlibay avatar Jun 12 '25 15:06 IAlibay

Thanks!

-- Oliver Beckstein (he/his/him) @.***

On Jun 12, 2025, at 08:34, Irfan Alibay @.***> wrote:

IAlibay left a comment (MDAnalysis/mdanalysis#5062) https://github.com/MDAnalysis/mdanalysis/pull/5062#issuecomment-2967322563 Who would start the process of getting on the cf feedstock recipe?

I have opened an issue on the feedstock, let's see what happens.

— Reply to this email directly, view it on GitHub https://github.com/MDAnalysis/mdanalysis/pull/5062#issuecomment-2967322563, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB2DHAVM53PF76OAREW7HT3DGM2DAVCNFSM6AAAAAB7BFF37KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNRXGMZDENJWGM. You are receiving this because you were mentioned.

orbeckst avatar Jun 12 '25 16:06 orbeckst

I have opened an issue on the feedstock, let's see what happens.

Thanks @IAlibay! I'd be OK to be added as a maintainer if needed.

RMeli avatar Jun 12 '25 16:06 RMeli

While we wait for the transformations people to respond so we can migrate, I have had a crack at updating the numpy C interface for transformations.c to modern NPY C API in #5068 such that things build correctly again.

Seems to be working?

hmacdope avatar Jun 23 '25 01:06 hmacdope

Thanks @hmacdope! Sounds good to me to fix the issue quickly with #5068 while we wait for the conda package to be sorted.

RMeli avatar Jun 23 '25 07:06 RMeli