Build wheels with numpy 1.25+ by IAlibay · Pull Request #4198 · MDAnalysis/mdanalysis

IAlibay · 2023-07-14T20:51:32Z

Fixes #4183

This will likely (silently) break all of CI since the order of things changes.

Things to check

azure pipelines
gh actions
cirrus (all good we don't have a specific numpy install)
deployment

PR Checklist

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

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

github-actions · 2023-07-14T20:54:09Z

Linter Bot Results:

Hi @IAlibay! Thanks for making this PR. We linted your code and found the following:

There are currently no issues detected! 🎉

codecov · 2023-07-14T21:05:54Z

Codecov Report

Patch coverage has no change and project coverage change: -0.21% ⚠️

Comparison is base (b21478f) 93.62% compared to head (a33cbd9) 93.41%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4198      +/-   ##
===========================================
- Coverage    93.62%   93.41%   -0.21%     
===========================================
  Files          179      183       +4     
  Lines        24199    23306     -893     
  Branches      4064     4064              
===========================================
- Hits         22657    21772     -885     
+ Misses        1026     1018       -8     
  Partials       516      516

see 24 files with indirect coverage changes

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

hmacdope

Looks good to me @IAlibay

IAlibay · 2023-08-09T00:00:46Z

Looks good to me @IAlibay

I mean it still needs a lot of changes / work.. I wouldn't prematurely approve it.

orbeckst

I don't feel knowledgeable to approve here — all I would do is look if the CI is still green. I assume that NEP29 allows us to depend on numpy >= 1.25?

IAlibay · 2023-08-09T00:11:23Z

I don't feel knowledgeable to approve here — all I would do is look if the CI is still green. I assume that NEP29 allows us to depend on numpy >= 1.25?

numpy 1.25 offers backwards compatibility all the way through to numpy 1.19 (which more than covers nep29).

What's left to do here in this PR is to go back and rework CI to do the right install order for wheel generation where necessary - i.e. install with 1.25 and then downgrade to 1.22 and see if it still works. I'll have a stab at it later this week probably.

IAlibay · 2023-08-14T20:23:43Z

Cron CI issue is elsewhere see #4238

IAlibay

Some comments for when folks get to review so it makes more sense what I'm doing here.

IAlibay · 2023-08-14T22:18:28Z

.github/actions/build-src/action.yaml

    description: 'build MDA docs'
    required: true
    default: false
+  isolation:


We're adding an option to trigger the pip --no-build-isolation flag. This will allow us to avoid pip trying to have its own isolated build & overidding already installed dependencies.

It took me a couple readings of this comment and the pip documentation to understand (if I have this correctly) that:

default (i.e. isolate) installs new build-time dependencies that can be separate from runtime dependencies or previously installed packages

--no-build-isolation does not override previously installed packages (I think?) but makes use of what is currently installed, which is why we added this flag here to test different build package versions

If that's right, could you please add that to the description of this flag, so later maintainers understand the use of the flag in our CI?

Honestly I'm not really 100% sure I know exactly how build isolation works, my take is that it creates an isolated environment with brand new everything to create wheels and then it gives you the wheels back in the world you first started in.

I'll try to add something sensible here.

Ok I've added a longer explanation and justification for how build isolation works, why we need it, etc...

I'm going to go ahead with the merge, but if you have the time to review what I wrote here and call out anything that's unclear in an issue that'd be great.

IAlibay · 2023-08-14T22:19:21Z

.github/workflows/deploy.yaml

+      - name: install_min_deps
+        if: "matrix.type == 'MIN'"
+        run: |
+          pip install pytest-xdist pytest-timeout


Moving this here because pip installing MDAnalysisTests should have already gotten pytest, you just need ot add pytest-xdist and pytest-timeout

.github/workflows/gh-ci-cron.yaml

IAlibay · 2023-08-14T22:19:47Z

.github/workflows/gh-ci-cron.yaml

      with:
        build-tests: true
        build-docs: false
+        isolation: false


turn off isolation here because we want to build with the nightly wheels

IAlibay · 2023-08-14T22:20:05Z

.github/workflows/gh-ci-cron.yaml

      with:
        build-tests: true
        build-docs: false
+        isolation: false


build with the deps you have installed

.github/workflows/gh-ci.yaml

IAlibay · 2023-08-14T22:21:20Z

.github/workflows/gh-ci.yaml

      with:
        build-tests: true
        build-docs: true
+        isolation: false


no need for isolation if we have all the dependencies here

azure-pipelines.yml

IAlibay · 2023-08-14T22:22:06Z

azure-pipelines.yml

    displayName: 'Build MDAnalysis'
    condition: and(succeeded(), eq(variables['BUILD_TYPE'], 'normal'))
+  - powershell: pip list
+    displayName: 'Check installed packages'


Keeping this around so we can get an idea of what's in the environment ahead of testing

IAlibay · 2023-08-14T22:22:47Z

package/pyproject.toml

+    # than NEP29 (see: https://numpy.org/doc/stable/dev/depending_on_numpy.html#adding-a-dependency-on-numpy)
+    # We pin our wheel installation to that and a maximum of numpy 2.0 since
+    # this will likely involve several breaking changes
+    "numpy>=1.25,<2.0; python_version>='3.9'",


NumPy 1.25+ is backwards compatible with its C/C++ API, so (as shown in the CI tests) we can build with it and then use an older version of NumPy at runtime and things just work.

IAlibay · 2023-08-15T03:55:26Z

Alright can I ask for a priority re-review & merge here @MDAnalysis/coredevs ? I would like to get this merged to get the 2.6.0 release out.

lilyminium

LGTM but IMO more documentation would be a huge help for future maintenance. Thanks for this @IAlibay

.github/workflows/gh-ci.yaml

lilyminium · 2023-08-15T08:01:25Z

.github/actions/build-src/action.yaml

    description: 'build MDA docs'
    required: true
    default: false
+  isolation:


It took me a couple readings of this comment and the pip documentation to understand (if I have this correctly) that:

default (i.e. isolate) installs new build-time dependencies that can be separate from runtime dependencies or previously installed packages

--no-build-isolation does not override previously installed packages (I think?) but makes use of what is currently installed, which is why we added this flag here to test different build package versions

If that's right, could you please add that to the description of this flag, so later maintainers understand the use of the flag in our CI?

IAlibay · 2023-08-15T10:26:56Z

Thanks for the quick review @lilyminium ! I'll go ahead and merge when CI returns green so we can move ahead with the release.

IAlibay · 2023-08-15T11:29:11Z

/azp run

azure-pipelines · 2023-08-15T11:29:19Z

Azure Pipelines successfully started running 1 pipeline(s).

build with numpy 1.25+

d54db2f

This was referenced Jul 16, 2023

Pin numpy to <2 ahead of breaking release #4201

Closed

Switch to scientifc python nightly package location for cron tests against upstream nightly wheels #4203

Merged

IAlibay added this to the 2.6.0 milestone Aug 8, 2023

IAlibay mentioned this pull request Aug 8, 2023

Release 2.6.x #4228

Closed

hmacdope approved these changes Aug 8, 2023

View reviewed changes

orbeckst reviewed Aug 9, 2023

View reviewed changes

IAlibay added 2 commits August 14, 2023 19:25

switch to pip install -e

21cecd1

some changes, some debugging

ebc8b95

github-actions bot added the Continuous Integration label Aug 14, 2023

IAlibay added 5 commits August 14, 2023 19:55

Merge branch 'develop' into numpy-1.25

0926a7c

fix typo in azp

6f93755

micromamba list

e5aa75c

add verbosity

d4abcb3

disable build isolation

2c48989

IAlibay added 3 commits August 14, 2023 21:27

add isolation flag

e00333a

be explicit about isolation (even though the default is ok)

520da67

actually pass the build isolation flag

717a6ce

IAlibay commented Aug 14, 2023

View reviewed changes

IAlibay added 5 commits August 15, 2023 00:24

inputs not input

edf9cb8

try to fix typo

2ebccb5

fix flag assignment

bb4b019

remove cron shim

2d04c4c

update changelog

2bd32ca

IAlibay marked this pull request as ready for review August 15, 2023 03:54

IAlibay changed the title ~~[wip] build wheels with numpy 1.25+~~ Build wheels with numpy 1.25+ Aug 15, 2023

lilyminium approved these changes Aug 15, 2023

View reviewed changes

Add comments on build isolation use

a33cbd9

IAlibay merged commit fa0fb82 into develop Aug 15, 2023

IAlibay deleted the numpy-1.25 branch August 15, 2023 15:41

IAlibay mentioned this pull request Aug 28, 2023

Dealing with NumPy build time requirements for the 2.6.x release #4260

Closed

IAlibay added the maintainability label Sep 21, 2023

Conversation

IAlibay commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

github-actions bot commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Linter Bot Results:

Uh oh!

codecov bot commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hmacdope left a comment

Choose a reason for hiding this comment

Uh oh!

IAlibay commented Aug 9, 2023

Uh oh!

orbeckst left a comment

Choose a reason for hiding this comment

Uh oh!

IAlibay commented Aug 9, 2023

Uh oh!

IAlibay commented Aug 14, 2023

Uh oh!

IAlibay left a comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAlibay commented Aug 15, 2023

Uh oh!

lilyminium left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAlibay commented Aug 15, 2023

Uh oh!

IAlibay commented Aug 15, 2023

Uh oh!

azure-pipelines bot commented Aug 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

IAlibay commented Jul 14, 2023 •

edited

Loading

github-actions bot commented Jul 14, 2023 •

edited

Loading

codecov bot commented Jul 14, 2023 •

edited

Loading