Modernise python package management:

[yakutovicha]

• Use flit instead of setuptools
• Replace setup.json & setup.py with pyproject.toml
• Update publish workflow
• Remove MANIFEST.in

[sphuber]

How does this work? Is this supposed to create the tag automatically? If so, this will never get called though, because the workflow is only run when a tag is pushed.

[yakutovicha]

I typically use bumpver to create a tag automatically. I will add the tool in the next PR together with a section in the README explaining how to use it.

[sphuber]

Ok, so the tag will have been created locally, and then the action is just responsible for creating a release entry on Github. However, did I understand correctly that this also pushes the build to PyPI? If so, then you don't want to have flit publish in the next step. Because then you are pushing to PyPI twice.

[yakutovicha]

However, did I understand correctly that this also pushes the build to PyPI?

No, publishing on PyPi is typically done as the next step, see for example https://github.com/nanotech-empa/aiida-gaussian/blob/master/.github/workflows/publish.yml

[sphuber]

Good practice to indicate explicitly which Python 3 versions are supported.

[yakutovicha]

Let's leave it for the next PR if that is fine.

[yakutovicha]

#168

[sphuber]

Does the identifier need to be quoted? That's what I have in my pyproject.toml's but not sure if it is required

[yakutovicha]

I don't know 🤷

[chrisjsewell]

Mainly looks good to me thanks, just a little change for the publish CI

[yakutovicha]

@sphuber I just checked the https://github.com/softprops/action-gh-release action, and it doesn't provide build-in dist folder generation. It only supports explicit file specifications.

However, as @chrisjsewell suggested, flit build does it. I will add this step to the workflow.

[sphuber]

@sphuber I just checked the https://github.com/softprops/action-gh-release action, and it doesn't provide build-in dist folder generation. It only supports explicit file specifications.

However, as @chrisjsewell suggested, flit build does it. I will add this step to the workflow.

Thanks for the links @yakutovicha . I might understand it incorrectly, but why are you calling the release action before the flit publish call? How can the release action attach the dist/* artifacts if they haven't been created yet by flit?

[yakutovicha]

Thanks for the links @yakutovicha . I might understand it incorrectly, but why are you calling the release action before the flit publish call? How can the release action attach the dist/* artifacts if they haven't been created yet by flit?

I placed

      - name: Build source distribution
        run: flit build

right before the gh-release action. It will create the dist/* artifacts. Do you know if the flit publish run creates them as well? If yes - then I can remove flit build and place the gh-release action right after flit publish.

[UPDATE]
Actually, I just learned that flit publish does create the dist folder 👍 . I will now update the PR.

[sphuber]

I see, maybe I was still being shown some old diff from the cache. Then it makes sense. Yes, moving the release action after the flit publish makes sense. In that case the release will also only be created if the publish went correctly

[sphuber]

One final difference between this workflow and what we use in other packages in aiidateam is that we run this step which is a sanity check to verify that the tag version matches the version of the package. This is to catch cases where people accidentally create the wrong tag version, or forgot to update the __version__ attribute. But maybe since you use bumpver and you trust that, you don't need it. Last thing, you might want to consider running the tests (and maybe pre-commit) before the publish step. If you only tag commits that have gone through PRs where the CI has run this might be a bit superfluous, but we do it anyway, just to be on the safe side. Typically the CD runs way less often compared to the CI, so that single extra build is not that big of a deal in times of required time.

[yakutovicha]

One final difference between this workflow and what we use in other packages in aiidateam is that we run this step which is a sanity check to verify that the tag version matches the version of the package. This is to catch cases where people accidentally create the wrong tag version, or forgot to update the __version__ attribute. But maybe since you use bumpver and you trust that, you don't need it.

This part is fully automated with the bumpver, so I don't think this is needed.

Last thing, you might want to consider running the tests (and maybe pre-commit) before the publish step. If you only tag commits that have gone through PRs where the CI has run this might be a bit superfluous, but we do it anyway, just to be on the safe side. Typically the CD runs way less often compared to the CI, so that single extra build is not that big of a deal in times of required time.

I also don't think this is needed. With bumpver, the releases are always done through the automatically generated commit (here is an example). And the only change is the version number.

Overall, I prefer simplicity when the advantage of adding more stuff is not super obvious.

[sphuber]

Overall, I prefer simplicity when the advantage of adding more stuff is not super obvious.

Yeah sure, I see that for your approach this makes perfect sense. Just thought I'd mention it for more context.