replaced lib.transformations with transformations package #5062

orbeckst · 2025-06-11T01:32:21Z

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

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!)

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/

- removed old lib.transformations code - use transformations package instead (which is the updated version of our old vendored code): updated imports everywhere - moved our only addition (transformations.rotaxis() to mdamath.rotaxis()) and left a deprecated version in lib.transformations - blackened files

Copilot

Pull Request Overview

This PR replaces the vendored lib.transformations C code with the external transformations package, migrates a custom rotaxis implementation into lib.mdamath, and leaves a deprecated wrapper in the old namespace.

Removed the compiled _transformations extension and switched all imports to use the transformations package.
Moved rotaxis into lib.mdamath and added a deprecation test for the old location.
Updated tests to call into the installed package and documented the change in the CHANGELOG.

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
testsuite/MDAnalysisTests/utils/test_transformations.py	Switched parameterized tests to use `t.transformations.*` and commented out obsolete Python-level tests
package/setup.py	Deleted the `MDAnalysis.lib._transformations` extension from build configuration
package/pyproject.toml	Added `transformations` to `dependencies` without pinning a version
package/MDAnalysis/lib/mdamath.py	Added a new `rotaxis` function with docs and versionchanged note
package/CHANGELOG	Recorded the replacement of vendored code and deprecation of `lib.transformations`

Comments suppressed due to low confidence (1)

testsuite/MDAnalysisTests/utils/test_transformations.py:399

Commenting out entire test blocks silently removes coverage; use pytest.mark.skip or xfail to indicate intentional deprecation while still reporting on coverage.

# @pytest.mark.parametrize(

testsuite/MDAnalysisTests/utils/test_transformations.py

package/pyproject.toml

codecov · 2025-06-11T06:47:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

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

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

orbeckst · 2025-06-11T07:46:27Z

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

+  transformations:
+    default: 'transformations'


I thought adding transformations here would install it as part of CI but I still see it as pypi installed https://github.com/MDAnalysis/mdanalysis/actions/runs/15578157334/job/43867072733?pr=5062#step:7:269

transformations 2025.1.1 pypi_0 pypi

What should I be doing?

I got time this weekend, will look into it!

Just to expand, I suspect that there is something deeper with the way in which conda recognises the package. We should check this before making the switch, otherwise we might end up with and rdkit pypi situation, except from a non-optional package.

One thing I noticed and maybe is relevant, is that the conda-forge package does not appear to be maintained: the latest packaged version is 2022.9.26.

sigh...

Do we want to put another orphan package on our shoulders (until we remove transformations in 3.0)?

Or do we want to go back and just vendor the 2025.x code?

It might be as easy as merging a bunch of PRs https://github.com/conda-forge/transformations-feedstock/pulls ... or not ;-)

I wouldn't say it is an orphan packaged, because PyPI is up to date and there has been a 2025 release. So I think the main issue is with the conda-forge feedstock, which we could take over?

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.

FYI: when I tried mamba install transformations in my developer env, it tried downgrading numpy. So I am just confirming what you've been saying: the conda-forge version of transformations needs to be updated for us to be useful.

Package Version Build Channel Size ───────────────────────────────────────────────────────────────────────── Install: ───────────────────────────────────────────────────────────────────────── + transformations 2022.9.26 py312hc7c0aa3_2 conda-forge 80kB Downgrade: ───────────────────────────────────────────────────────────────────────── - numpy 2.2.6 py312h72c5963_0 conda-forge Cached + numpy 1.26.4 py312heda63a1_0 conda-forge Cached

RMeli

Thanks @orbeckst, always great to remove so much code without losing functionality.

maintainer/conda/environment.yml

package/pyproject.toml

testsuite/MDAnalysisTests/utils/test_transformations.py

IAlibay

I'm going to put a soft block so that we can work out what's up with the pypi install taking precedence over conda. Just so we don't see another rdkit-style pypi issue.

orbeckst · 2025-06-12T14:57:47Z

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: ***@***.***>

IAlibay · 2025-06-12T15:34:36Z

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.

orbeckst · 2025-06-12T16:00:29Z

Thanks!

…

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

On Jun 12, 2025, at 08:34, Irfan Alibay ***@***.***> wrote: IAlibay left a comment (MDAnalysis/mdanalysis#5062) <#5062 (comment)> 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 <#5062 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2DHAVM53PF76OAREW7HT3DGM2DAVCNFSM6AAAAAB7BFF37KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNRXGMZDENJWGM>. You are receiving this because you were mentioned.

RMeli · 2025-06-12T16:15:40Z

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.

hmacdope · 2025-06-23T01:23:22Z

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?

RMeli · 2025-06-23T07:52:02Z

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

orbeckst force-pushed the remove-transformations branch from 9dc6308 to 83a8e77 Compare June 11, 2025 01:41

orbeckst added 2 commits June 10, 2025 18:51

doc updates

f38de12

updated CHANGELOG

1a5a844

orbeckst requested a review from Copilot June 11, 2025 01:55

This comment was marked as outdated.

Sign in to view

orbeckst requested a review from Copilot June 11, 2025 03:36

Copilot AI reviewed Jun 11, 2025

View reviewed changes

testsuite/MDAnalysisTests/utils/test_transformations.py Outdated Show resolved Hide resolved

package/pyproject.toml Show resolved Hide resolved

fixed tests

72c64c8

orbeckst force-pushed the remove-transformations branch from 25f8085 to 72c64c8 Compare June 11, 2025 06:34

add transformations to CI as dependency

1ca09f5

orbeckst commented Jun 11, 2025

View reviewed changes

orbeckst requested review from RMeli, IAlibay and ljwoods2 June 11, 2025 15:41

RMeli reviewed Jun 11, 2025

View reviewed changes

maintainer/conda/environment.yml Outdated Show resolved Hide resolved

package/pyproject.toml Outdated Show resolved Hide resolved

testsuite/MDAnalysisTests/utils/test_transformations.py Outdated Show resolved Hide resolved

no version boundary on transformations

ce4f94e

orbeckst force-pushed the remove-transformations branch from 213310f to 4df6847 Compare June 11, 2025 20:18

IAlibay requested changes Jun 11, 2025

View reviewed changes

moved rotaxis() tests to tests for mdamath

bd88561

orbeckst force-pushed the remove-transformations branch from 4df6847 to bd88561 Compare June 11, 2025 20:44

orbeckst assigned RMeli Jun 12, 2025

replaced lib.transformations with transformations package #5062

Are you sure you want to change the base?

replaced lib.transformations with transformations package #5062

Uh oh!

Conversation

orbeckst commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Developers Certificate of Origin

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RMeli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IAlibay left a comment

Choose a reason for hiding this comment

Uh oh!

orbeckst commented Jun 12, 2025 via email

Uh oh!

IAlibay commented Jun 12, 2025

Uh oh!

orbeckst commented Jun 12, 2025 via email

Uh oh!

RMeli commented Jun 12, 2025

Uh oh!

hmacdope commented Jun 23, 2025

Uh oh!

RMeli commented Jun 23, 2025

Uh oh!

Uh oh!

orbeckst commented Jun 11, 2025 •

edited

Loading

codecov bot commented Jun 11, 2025 •

edited

Loading