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

Migrate to pyproject.toml #554

Merged
merged 1 commit into from
Jul 5, 2024
Merged

Conversation

dbast
Copy link
Contributor

@dbast dbast commented May 4, 2024

Description

Move towards a more modernised tool-agnostic Python package setup: Migrate .coveragerc, MANIFEST.in, pytest.ini, setup.py to one single pyproject.toml to have less boilerplate files in the repo root.

This is not so much relevant for the use on the image as there only the src folder is kept from the repo and every other file is ignored / removed see e.g. https://github.com/SeedSigner/seedsigner/blob/dev/.github/workflows/build.yml#L76

So the pyproject.toml is both an informal place containing all the Python tooling specific package information and is currently only used when installing the package during testing via pip. Both coverage (since version 5.0, we use 7.3.1) and pytest (since version 6.0, we use 7.4.2) support reading their configuration from the central pyproject.toml.

The formal spec can be found here https://packaging.python.org/en/latest/specifications/pyproject-toml/#pyproject-toml-spec

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms/os:

Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.

@dbast dbast marked this pull request as draft May 4, 2024 06:22
@dbast dbast marked this pull request as ready for review May 4, 2024 06:33
pyproject.toml Outdated
name = "seedsigner"
readme = "README.md"
requires-python = ">=3.10"
version = "0.7.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize this was previously in setup.py, but I wonder if there's any way to mitigate the minor housekeeping required to keep this in sync with releases.

Can version just be omitted? We're not going to be releasing this as a pip package so specifying version here seems pointless (unless required by spec).

Copy link
Contributor Author

@dbast dbast Jul 4, 2024

Choose a reason for hiding this comment

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

Rebased the PR so that the version in the pyproject.toml file is now 0.8.0-rc1 like on the current dev branch.

Removing the version entirely is not possible as it results in the error configuration error: `project` must contain ['version'] properties` when pip installing or creating a sdist via `python -b build ..

Another option would to calculate the version dynamically based on repo tags via setuptools-scm... maybe something for another PR, as it is a logical change and not just moving config settings into the pyproject.toml.

@kdmukai
Copy link
Contributor

kdmukai commented Jul 3, 2024

ACK!

I can confirm that everything ran as expected:

pytest
coverage run -m pytest
coverage report
coverage html

@newtonick
Copy link
Collaborator

ACK

@newtonick newtonick merged commit 49c4a43 into SeedSigner:dev Jul 5, 2024
1 check passed
@dbast dbast deleted the pyproject.toml branch July 5, 2024 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged Not Yet Released
Development

Successfully merging this pull request may close these issues.

3 participants