Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Github action to publish to pypi and test pypi #266

Closed
wants to merge 1 commit into from
Closed

Github action to publish to pypi and test pypi #266

wants to merge 1 commit into from

Conversation

cbanek
Copy link
Contributor

@cbanek cbanek commented Jun 12, 2021

Fixes #247. Also fixes #181 .

Adds a github publish hook that should run if we make a tag. Also updated the release instructions

@codecov
Copy link

codecov bot commented Jun 12, 2021

Codecov Report

Merging #266 (1ffe577) into master (dd3914e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #266   +/-   ##
=======================================
  Coverage   74.95%   74.95%           
=======================================
  Files          44       44           
  Lines        5063     5063           
=======================================
  Hits         3795     3795           
  Misses       1268     1268           

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 dd3914e...1ffe577. Read the comment docs.

@eteq
Copy link
Member

eteq commented Jun 18, 2021

Just a thought here: over in the astropy core package (and in the sunpy core), we build wheels using some machinery that's also partly shared with other corners of the Python world. See https://github.com/astropy/astropy/blob/main/azure-pipelines.yml for an example of how its implemented or just look at the docs for the pipeline. It does have the awkwardness that it requires setting up Azure pipelines, though, which is not that hard but is another CI to deal with.

Also, I'm not sure pyvo really needs wheels anyway since it's pure python? But in the event that there's some binaries needed in the future this machinery might be worth thinking about.

@andamian
Copy link
Contributor

Just a thought here: over in the astropy core package (and in the sunpy core), we build wheels using some machinery that's also partly shared with other corners of the Python world. See https://github.com/astropy/astropy/blob/main/azure-pipelines.yml for an example of how its implemented or just look at the docs for the pipeline. It does have the awkwardness that it requires setting up Azure pipelines, though, which is not that hard but is another CI to deal with.

Also, I'm not sure pyvo really needs wheels anyway since it's pure python? But in the event that there's some binaries needed in the future this machinery might be worth thinking about.

No need for binaries at the moment but it's good to know that there's a solution if we ever need it. Thanks @eteq

@@ -34,6 +34,7 @@ exclude = extern,sphinx,*parsetab.py
package_name = pyvo
description = Astropy affiliated package for accessing Virtual Observatory data and services
long_description =
long_description_content_type = text/x-rst
author = the IVOA community
author_email = sbecker@ari.uni-heidelberg.de
Copy link
Contributor

Choose a reason for hiding this comment

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

This E-mail is not valid anymore. Probably best to put a astropy.org address?

@pllim pllim added the Build wheels Run publish workflow for PR label Jan 11, 2022
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Here are my comments, assuming #285 and #287 get done first before this PR.

name: Publish to pypi

on:
push:
Copy link
Member

Choose a reason for hiding this comment

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

You want to be more selective on push (i.e., only when push to main and only when pushing tags). Also, enabling it for PRs is also nice to ensure a PR won't break the release wheel (more below).

Suggested change
push:
pull_request:
push:
branches:
- main
tags:
- '*'

@@ -0,0 +1,56 @@
name: Publish to pypi
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name: Publish to pypi
name: Publish to PyPI

jobs:
build-n-publish:
name: Build and publish Python 🐍 distributions 📦 to PyPI
runs-on: ubuntu-latest
Copy link
Member

Choose a reason for hiding this comment

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

The if here will limit PR runs to only those labeled with Build wheels label, which only a maintainer can apply. This will safeguard against unnecessary PR runs. I already added the label to the repo.

Suggested change
runs-on: ubuntu-latest
runs-on: ubuntu-latest
if: (github.event_name == 'push' || contains(github.event.pull_request.labels.*.name, 'Build wheels'))

python-version: 3.8

- name: Install python-build and twine
run: python -m pip install build "twine>=3.3"
Copy link
Member

Choose a reason for hiding this comment

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

Sometimes the Action complains about outdated pip, which really bothers me for some reason, hence this:

Suggested change
run: python -m pip install build "twine>=3.3"
run: python -m pip install pip build "twine>=3.3" -U

- name: List result
run: ls -l dist

- name: Check long_description
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Check long_description
- name: Check dist

Comment on lines +44 to +49
- name: Check version is valid
if: startsWith(github.ref, 'refs/tags')
run: |
REPO_VERSION=`echo "$GITHUB_REF" | sed -E 's,/refs/tags/,,'`
grep "^version = $REPO_VERSION" setup.cfg || (echo "Version in tag $REPO_VERSION does not match version in setup.cfg" && exit -1)

Copy link
Member

Choose a reason for hiding this comment

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

This step will not be needed after #285 because then setuptools_scm would auto-generate the version from tag for you. There won't be any need to manually bump any version.

Suggested change
- name: Check version is valid
if: startsWith(github.ref, 'refs/tags')
run: |
REPO_VERSION=`echo "$GITHUB_REF" | sed -E 's,/refs/tags/,,'`
grep "^version = $REPO_VERSION" setup.cfg || (echo "Version in tag $REPO_VERSION does not match version in setup.cfg" && exit -1)

Comment on lines +13 to +16
2. Use the GitHub releases to draft a new release, and put the version
number in for the tag. This tag must match what is in the setup.cfg
(without the dev). This will trigger a github action that builds
the release and uploads it to pypi.
Copy link
Member

Choose a reason for hiding this comment

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

This step needs #287 to be resolved first to work properly.

Suggested change
2. Use the GitHub releases to draft a new release, and put the version
number in for the tag. This tag must match what is in the setup.cfg
(without the dev). This will trigger a github action that builds
the release and uploads it to pypi.
4. Create a GitHub Release off your new release tag and publish it.
Check Zenodo DOI to make sure it got pushed properly.


4. git tag -a version -m "releasing new version version" (this makes a release tag)
3. Make a PR with the following changes to begin the new release cycle:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
3. Make a PR with the following changes to begin the new release cycle:
5. Make the following changes to begin the new release cycle (either push directly to `main` or as a PR):

8. Edit setup.cfg and set the version to the next release number and add .dev after the version number. Add a new section at the top for the next release number

9. Commit and push. This begins the new release
- Edit setup.cfg and set the version to the next release number plus ".dev"
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to edit setup.cfg after #285 is done unless its default scheme for dev tagging is not what you want. See https://github.com/pypa/setuptools_scm#version-number-construction for more info.

Suggested change
- Edit setup.cfg and set the version to the next release number plus ".dev"


9. Commit and push. This begins the new release
- Edit setup.cfg and set the version to the next release number plus ".dev"
- Add a new section at the top of CHANGES.rst for the next release number
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Add a new section at the top of CHANGES.rst for the next release number
- Add a new section at the top of CHANGES.rst for the next release number and append "(unreleased)" to it.
- Move open issues/PRs to next milestone. Close the release milestone.

@pllim pllim added this to the v1.3 milestone Jan 11, 2022
@pllim
Copy link
Member

pllim commented Feb 15, 2022

#290 is merged. pyvo now adheres to APE 17. Please update this PR to reflect that. Thanks!

@tomdonaldson
Copy link
Contributor

As some of this is now out of date with respect to APE-17, I'm going to close this with the suggestion that we try again to address #247 and #181 prior to the next release. The structure and approach here and the review comments will be excellent resources for that attempt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build wheels Run publish workflow for PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publish to PyPI using GitHub Actions Simplify the release process
6 participants