Skip to content

Enable automated wheel and dist uploads on releases#3680

Merged
richardjgowers merged 30 commits intodevelopfrom
wheels-on-the-bus
Jun 1, 2022
Merged

Enable automated wheel and dist uploads on releases#3680
richardjgowers merged 30 commits intodevelopfrom
wheels-on-the-bus

Conversation

@IAlibay
Copy link
Copy Markdown
Member

@IAlibay IAlibay commented May 26, 2022

Fixes #1300

Changes made in this Pull Request:

  • Added a deploy action that should build wheels using cibuildwheel, and sdist and then upload them to testpypi / pypi on specific triggers
  • Action runs on every push (so we can notice if something goes wrong), but push to testpypi only happens when you create a tag, and push to pypi only happens when you create a release
  • Created a deploy environment with the necessary pypi / testpypi API token secrets
  • Added maintainer script to normalize release versions in the same way setuptools does (so we can pick it up from testpypi)
  • Standard CI has been expanded to do a full deps check of py3.10 (instead of the minimum deps check done previously)

TODO:

  • Docs

PR Checklist

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

@codecov
Copy link
Copy Markdown

codecov bot commented May 27, 2022

Codecov Report

Merging #3680 (c03ac8d) into develop (1945878) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #3680   +/-   ##
========================================
  Coverage    94.32%   94.33%           
========================================
  Files          191      191           
  Lines        24917    24917           
  Branches      3357     3357           
========================================
+ Hits         23503    23505    +2     
+ Misses        1365     1364    -1     
+ Partials        49       48    -1     
Impacted Files Coverage Δ
package/MDAnalysis/converters/RDKit.py 98.74% <0.00%> (+0.31%) ⬆️
package/MDAnalysis/converters/RDKitParser.py 96.96% <0.00%> (+0.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1945878...c03ac8d. Read the comment docs.

@IAlibay IAlibay changed the title [WIP] enable automated wheel and dist uploads on releases Enable automated wheel and dist uploads on releases May 29, 2022
@IAlibay IAlibay added this to the 2.2.0 milestone May 29, 2022
@IAlibay IAlibay temporarily deployed to deploy May 29, 2022 16:35 Inactive
@IAlibay IAlibay temporarily deployed to deploy May 29, 2022 17:21 Inactive
@IAlibay
Copy link
Copy Markdown
Member Author

IAlibay commented May 29, 2022

Ok this should be fine now, I've tested the testpypi portion of the release and wheels were built fine here: https://test.pypi.org/project/MDAnalysisTests/2.2.0.dev0/#modal-close

@MDAnalysis/coredevs this needs reviewing as a priority, it's necessary for the 2.2.0 release. It should make our life a lot easier going forward too!

@Seanny123 - I think this should fix the wheel issue, but if you can have a look at if this meets your needs that would be great.

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.

This is great DevOps work, @IAlibay . I have some minor questions because I am not that well versed in all the GH-actions magic and I am happy to positively re-review soon.

The main think I am looking for is minimal maintainer docs that explain the release process and outline the thinking behind the changes, i.e., in plain language, what are the goals and gotchas?

@IAlibay
Copy link
Copy Markdown
Member Author

IAlibay commented May 29, 2022

Ok, on top of updating the release notes in the wiki I've also opened up this PR in the userguide @orbeckst: MDAnalysis/UserGuide#208

I would prefer not have to duplicate docs a third time, so I might suggest that from now on all maintainer docs go in the userguide.

@Seanny123
Copy link
Copy Markdown

@Seanny123 - I think this should fix the wheel issue, but if you can have a look at if this meets your needs that would be great.

This definitely meets my needs. Previously, I was also posting about trying to build the wheels with minimal Numpy versions, but I am no longer concerned about that, even if I can't tell from this PR the compatibility of the wheels being built.

@IAlibay
Copy link
Copy Markdown
Member Author

IAlibay commented May 29, 2022

@Seanny123 - I think this should fix the wheel issue, but if you can have a look at if this meets your needs that would be great.

This definitely meets my needs.

Awesome. Sorry it took so long to get done :( Our release methods are a bit overly complicated.

Previously, I was also posting about trying to build the wheels with minimal Numpy versions, but I am no longer concerned about that, even if I can't tell from this PR the compatibility of the wheels being built.

So it should be pulling from our pyproject.toml and using the minimum compatible versions of numpy for ABI compatibility purposes.

That being said, I agree that it's pretty annoying that it's not clear from the action what's going on. Let me try re-enabling it with a higher verbosity level, maybe that will tell us what it's doing.

@IAlibay
Copy link
Copy Markdown
Member Author

IAlibay commented May 29, 2022

Ok yeah, it is respecting pyproject.toml (hence the 1.19.3 pick): https://github.com/MDAnalysis/mdanalysis/runs/6645750506?check_suite_focus=true#step:3:191

I'll leave verbosity at level 1 so we can debug better if something goes wrong :)

@Seanny123
Copy link
Copy Markdown

I appreciate the increased verbosity. This will help me onboard other projects of this type (mdtraj in particular) to a CI release pipeline.

Sorry it took so long to get done :( Our release methods are a bit overly complicated.

Your methods are perfectly understandable given the constraints! Making Python fast is non-trivial and makes packaging significantly harder. I'm actually super impressed you got it done at all, let alone so quickly!

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.

Thank you for addressing all my questions and concerns @IAlibay ; I am approving from my end.

However, it would certainly be good if some other @MDAnalysis/coredevs could spare a look.

@IAlibay
Copy link
Copy Markdown
Member Author

IAlibay commented May 31, 2022

Thank you for addressing all my questions and concerns @IAlibay ; I am approving from my end.

However, it would certainly be good if some other @MDAnalysis/coredevs could spare a look.

@MDAnalysis/coredevs I'm going to merge this in ~ 24h unless there are any further comments here.

@richardjgowers richardjgowers merged commit c9105da into develop Jun 1, 2022
@richardjgowers richardjgowers deleted the wheels-on-the-bus branch June 1, 2022 09:29
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.

build binary .whl packages ?

4 participants