Skip to content

Modernize python packaging #851

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

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

ddelange
Copy link
Collaborator

@ddelange ddelange commented Mar 6, 2025

Please pick a concise, informative and complete title for your PR.

The title is important because it will appear in our change log.

Motivation

Fix #859

Release is failing.

  • move to trusted publishing in release.yml
  • move to GitHub Release trigger in release.yml (good security for trusted publishing: git release permission vs general push permission)
  • add a dependabot.yml to check for github actions updates monthly (opens PRs to bump actions/setup-python etc if available)
  • deprecate setup.py and version.py in favour of pyproject.toml and setuptools_scm for git-based versioning
    • consequently moved tests out of the smart_open folder using rename commit to retain blame (as we don't want tests in the whl/tar.gz)
    • you can keep using v7.... as git tags prefix, it will be automatically stripped by setuptools_scm at runtime.
    • no more need to push version bump commits. just release your Github Release and let it publish with that new tag you just created
  • Simplify release workflow README.md

a small demo of the versioning scheme of setuptools_scm:

$ git status
On branch pypa-publish
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
    modified:   .github/workflows/release.yml
    modified:   pyproject.toml
    deleted:    setup.py
    modified:   smart_open/__init__.py
    deleted:    smart_open/version.py

$ git lg
* c299079 (HEAD -> pypa-publish, upstream/develop, origin/develop, origin/HEAD, develop) bump version to 7.2.0.dev0
*   e271ce6 Merge branch 'master' into develop
|\
| *   a08f0a0 (tag: v7.2.0, upstream/master) Merge branch 'release-7.2.0'
...

$ pip install -qe .
$ python -c 'import smart_open; print(smart_open.__version__)'
6.4.1.dev83+gc299079.d20250306

$ git pull upstream --tags
remote: Enumerating objects: 18, done.
remote: Counting objects: 100% (10/10), done.
remote: Compressing objects: 100% (3/3), done.
remote: Total 18 (delta 7), reused 9 (delta 7), pack-reused 8 (from 2)
Unpacking objects: 100% (18/18), 15.92 KiB | 2.27 MiB/s, done.
From https://github.com/RaRe-Technologies/smart_open
 * [new branch]      fix-ssh    -> upstream/fix-ssh
   299ca52..a08f0a0  master     -> upstream/master
 * [new tag]         v7.0.0     -> v7.0.0
 * [new tag]         v7.0.1     -> v7.0.1
 * [new tag]         v7.0.2     -> v7.0.2
 * [new tag]         v7.0.3     -> v7.0.3
 * [new tag]         v7.0.4     -> v7.0.4
 * [new tag]         v7.0.5     -> v7.0.5
 * [new tag]         v7.1.0     -> v7.1.0
 * [new tag]         v7.2.0     -> v7.2.0
You asked to pull from the remote 'upstream', but did not specify
a branch. Because this is not the default configured remote
for your current branch, you must specify a branch on the command line.

$ pip install -qe .
$ python -c 'import smart_open; print(smart_open.__version__)'
7.2.1.dev2+gc299079.d20250306

$ cd ~ && pip install -q https://github.com/ddelange/smart_open/releases/download/v7.2.1.rc6/smart_open-7.2.1rc6-py3-none-any.whl

$ python -c 'import smart_open; print(smart_open.__version__)'
7.2.1rc6

Please explain the motivation behind this PR.

If you're fixing a bug, link to the issue using a supported keyword like "Fixes #{issue_number}".

If you're adding a new feature, then consider opening a ticket and discussing it with the maintainers before you actually do the hard work.

Tests

Only delivery plumbing.

If you're fixing a bug, consider test-driven development:

  1. Create a unit test that demonstrates the bug. The test should fail.
  2. Implement your bug fix.
  3. The test you created should now pass.

If you're implementing a new feature, include unit tests for it.

Make sure all existing unit tests pass.
You can run them locally using:

pytest smart_open

If there are any failures, please fix them before creating the PR (or mark it as WIP, see below).

Work in progress

If you're still working on your PR, mark the PR as draft PR.

We'll skip reviewing it for the time being.

Once it's ready, mark the PR as "ready for review", and ping one of the maintainers (e.g. mpenkov).

Checklist

Before you mark the PR as "ready for review", please make sure you have:

  • Picked a concise, informative and complete title
  • Clearly explained the motivation behind the PR
  • Linked to any existing issues that your PR will be solving
  • Included tests for any new functionality
  • Checked that all unit tests pass

Workflow

Please avoid rebasing and force-pushing to the branch of the PR once a review is in progress.

Rebasing can make your commits look a bit cleaner, but it also makes life more difficult from the reviewer, because they are no longer able to distinguish between code that has already been reviewed, and unreviewed code.

@ddelange
Copy link
Collaborator Author

ddelange commented Mar 6, 2025

@mpenkov apart from the TODO below, this should be good to go

@mpenkov can you please add piskvorky/smart_open plus release.yml as trusted publisher here?

ref trusted publishing

then I can do the plumbing in release.yml and I'll need to deprecate setup.py in favour of pyproject.toml. once that merges to master, you'll be unblocked

ref #848 (comment)

@ddelange ddelange force-pushed the pypa-publish branch 9 times, most recently from 5dbee20 to 0b7b5af Compare March 6, 2025 21:34
@ddelange
Copy link
Collaborator Author

ddelange commented Mar 6, 2025

release looks good on my fork, apart from that it fails because my fork is not a trusted publisher of course.

release.yml will only get triggered once for a release, namely when the Release is (pre)released. Edits to release description etc won't trigger the workflow.

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 8, 2025

Can you please update the release/ subdirectory? What's the new process for making a new release?

Comment on lines 20 to 37
![New release](https://github.com/user-attachments/assets/cf8f2fa4-37c1-4e50-9fd8-ab6e3fd705b5)

- Check that the [latest commit](https://github.com/piskvorky/smart_open/commits/develop) on `develop` passed all CI.
- Make sure you're on `master` and you're up to date:
- `git checkout master && git pull`
- Merge `develop` into `master`.
- `git pull origin develop --no-ff --no-edit && git push`
- Draft a [new release](https://github.com/piskvorky/smart_open/releases/new).
- Fill in the new tag + enter.
- Confirm that it reads "Excellent! This tag will be created from the target when you publish this release.".
- Select target branch `master`.
- Click "Generate release notes" on the right top.
- Keep the tab open.
- Copy the generated bullet points into `CHANGELOG.md`.
- Commit `CHANGELOG.md` to `master` and push:
- `git checkout master && git pull && git add CHANGELOG.md && git commit -m "Update CHANGELOG.md" && git push`
- Click "Publish release".
- Github Actions `release.yml` is triggered, and uploads distributions to PyPI and to the Github Release.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mpenkov how does this look? rendered version

Copy link
Collaborator Author

@ddelange ddelange Mar 8, 2025

Choose a reason for hiding this comment

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

multiple follow up commits, please look at the rendered version :)

Copy link
Collaborator Author

@ddelange ddelange Mar 8, 2025

Choose a reason for hiding this comment

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

I tried to reduce cognitive complexity as much as possible:

  • Added a new update_changelog.py which iterates over the diff and prepends a new section to CHANGELOG.md in the same format as usual.
  • The rest of the versioning is now fully automatic: 4 scripts less, no more copy pasting.

@ddelange ddelange mentioned this pull request Mar 14, 2025
@mpenkov
Copy link
Collaborator

mpenkov commented Mar 29, 2025

I'm a bit busy with my life outside of open source at the moment, but I'll have a look at this as soon as I get some spare cycles.

Copy link
Collaborator Author

@ddelange ddelange Mar 29, 2025

Choose a reason for hiding this comment

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

deleted here as part of release folder clean up (wasn't and won't be used during release flow) and opened #853

Comment on lines -35 to -38
tests (package)
transport
utils
version
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

first time #853 comes in handy: just pulled develop and lint started failing on this PR :)


- Check that the [latest commit](https://github.com/piskvorky/smart_open/commits/develop) on `develop` passed all CI.
- Make sure you're on `develop` and you're up to date locally:
- `git checkout develop && git pull`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we reduce the number of manual steps and copy/pasting here? Previously, releasing was a simple matter of calling prepare.sh and merge.sh and following the on-screen instructions, with occasional reference to this README.md.

Copy link
Collaborator Author

@ddelange ddelange May 18, 2025

Choose a reason for hiding this comment

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

fdd7e2e adds release/release.sh with chmod +x 👍

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.

Please release 7.2.0 to PyPI
2 participants