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

Add automated pypi release support #196

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

kunikachandra
Copy link

@kunikachandra kunikachandra commented Mar 6, 2024

This PR will partially solve (the pypi part) issue #46. After this the selector package will be accessible via pypi pip.

  1. The project owners will need to 'Add a new pending publisher' in pypi https://pypi.org/manage/account/publishing/ with the name selector or qc-selector (pyproject.toml line 26 needs to be changed to qc-selector if we want pip install qc-selector to work).
  2. Create a new API token under https://pypi.org/manage/account/
  3. In the GitHub settings create a new environment secret with the variable name PYPI_API_TOKEN and paste the API token.
  4. Create a new release to trigger the package build and publish to pypi.

@kunikachandra kunikachandra force-pushed the pypi-release branch 2 times, most recently from 3d37ed4 to b156e16 Compare March 6, 2024 19:17
@kunikachandra
Copy link
Author

@FanwangM can you take a look? Sorry for force pushing couple of times, had some pre-commit ci failures

@FanwangM
Copy link
Collaborator

FanwangM commented Mar 6, 2024

Thanks for pushing this forward. I will look at it later. @kunikachandra

Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.21%. Comparing base (dbc2202) to head (6cf18ff).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #196   +/-   ##
=======================================
  Coverage   96.21%   96.21%           
=======================================
  Files           9        9           
  Lines         951      951           
=======================================
  Hits          915      915           
  Misses         36       36           

Copy link
Collaborator

@FanwangM FanwangM 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 the PR for releasing of our package. I have put some comments for your reference. @kunikachandra

jobs:
deploy:

runs-on: ubuntu-latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

We will need support from Linux, MacOS, and Windows systems.

Copy link
Author

Choose a reason for hiding this comment

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

Do we need to test on multiple versions of each OS or testing on the latest version of each OS will be sufficient?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Testing on the latest version for each OS would be sufficient.

Copy link
Author

Choose a reason for hiding this comment

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

While uploading to pypi do we need to upload wheels of all possible combinations of python versions with OS, or just 1 Source distribution(tar.gz) and 1 Built distribution (.whl)?
And, which python versions are we testing on?

Copy link
Author

Choose a reason for hiding this comment

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

@FanwangM if we want wheels for all the 3 OS, then we would have to change the setup.py file to output the wheels with dynamic naming

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please help change setup.py? Thanks. @kunikachandra

Copy link
Author

Choose a reason for hiding this comment

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

Yes, sure! I will start working on it once my exams are over in 2-3 days

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's good to use the latest version,

- uses: actions/checkout@v4

according to https://github.com/actions/checkout.

steps:
- uses: actions/checkout@v3
- name: Set up Python
uses: actions/setup-python@v3
Copy link
Collaborator

Choose a reason for hiding this comment

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

We would go for

uses: actions/setup-python@v5

https://github.com/actions/setup-python

Comment on lines +31 to +35
sudo apt-get update
# scipy dependencies
sudo apt-get install -y libopenblas-dev
python -m pip install --upgrade pip
pip install build
Copy link
Collaborator

Choose a reason for hiding this comment

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

We will need to make this work for Windows and MacOs systems.

- name: Build package
run: python -m build
- name: Publish package
uses: pypa/gh-action-pypi-publish@27b31702a0e7fc50959f5ad993c78deac1bdfc29
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe

uses: pypa/[email protected]

?

pyproject.toml Show resolved Hide resolved
@kunikachandra
Copy link
Author

@FanwangM should I create a new PR for setup.py or commit in this PR only?

Also, should I change the name to qc-selector instead of selector for the pypi package because selector is already taken by a previous project?

@FanwangM
Copy link
Collaborator

@FanwangM should I create a new PR for setup.py or commit in this PR only?

Also, should I change the name to qc-selector instead of selector for the pypi package because selector is already taken by a previous project?

You can keep adding new things into this PR. And we would prefer using the qc-selector. @kunikachandra
Thanks for helping with this.

@@ -82,3 +81,17 @@
# zip_safe=False,
# todo: add classifiers
)

# Naming format of the pypi wheel
wheel_name_format = "{name}-{version}-{py_version}-none-any.whl"
Copy link
Author

Choose a reason for hiding this comment

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

@FanwangM currently I am using naming conventions that will generate packages like this qc_selector-0.0.1-py3-none-any.whl. I used streamlit's naming conventions as a reference. If you want some other format, let me know, I will update it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants