-
Notifications
You must be signed in to change notification settings - Fork 22
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
base: main
Are you sure you want to change the base?
Conversation
3d37ed4
to
b156e16
Compare
b156e16
to
87a7d94
Compare
@FanwangM can you take a look? Sorry for force pushing couple of times, had some pre-commit ci failures |
Thanks for pushing this forward. I will look at it later. @kunikachandra |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
.github/workflows/python-publish.yml
Outdated
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- uses: actions/checkout@v3 |
There was a problem hiding this comment.
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.
.github/workflows/python-publish.yml
Outdated
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Set up Python | ||
uses: actions/setup-python@v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sudo apt-get update | ||
# scipy dependencies | ||
sudo apt-get install -y libopenblas-dev | ||
python -m pip install --upgrade pip | ||
pip install build |
There was a problem hiding this comment.
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.
.github/workflows/python-publish.yml
Outdated
- name: Build package | ||
run: python -m build | ||
- name: Publish package | ||
uses: pypa/gh-action-pypi-publish@27b31702a0e7fc50959f5ad993c78deac1bdfc29 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FanwangM should I create a new PR for Also, should I change the name to |
You can keep adding new things into this PR. And we would prefer using the |
@@ -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" |
There was a problem hiding this comment.
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!
This PR will partially solve (the pypi part) issue #46. After this the
selector
package will be accessible via pypi pip.selector
orqc-selector
(pyproject.toml
line 26 needs to be changed toqc-selector
if we wantpip install qc-selector
to work).PYPI_API_TOKEN
and paste the API token.