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

feat: Migrate to PEP 621 compliant pyproject.toml format #5569

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

Conversation

zarch
Copy link
Contributor

@zarch zarch commented Feb 21, 2025

This commit migrates the pyproject.toml file to be fully compliant with PEP 621, enabling interoperability with tools like uv, pip, and other standard Python packaging tools, while maintaining full compatibility with Poetry.

Key changes:

  • Added [project] table: Moved core project metadata (name, version, description, authors, license, classifiers, dependencies, optional dependencies, and requires-python) to the standardized [project] table as specified by PEP 621. This includes using PEP 508 strings for dependencies.
  • Maintained [build-system]: Kept the [build-system] table pointing to Poetry's build backend, ensuring that pip (and thus uv) can correctly build the package from source.
  • Moved [tool.ruff]: Moved the [tool.ruff] configuration higher in the file for better organization.
  • Refactored project urls: Correctly use the [project.urls] table, as specified by PEP 621.

This change allows developers to choose between using Poetry or other dependency management and installation tools. Poetry can still be used for building and publishing, leveraging its existing tooling.

Please review the following checklist.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md

This commit migrates the `pyproject.toml` file to be fully compliant with
PEP 621, enabling interoperability with tools like `uv`, `pip`, and other
standard Python packaging tools, while maintaining full compatibility with
Poetry.

Key changes:

- **Added `[project]` table:** Moved core project metadata (name, version,
  description, authors, license, classifiers, dependencies, optional
  dependencies, and requires-python) to the standardized `[project]` table
  as specified by PEP 621.  This includes using PEP 508 strings for
  dependencies.
- **Maintained `[build-system]`:** Kept the `[build-system]` table pointing to
  Poetry's build backend, ensuring that `pip` (and thus `uv`) can correctly
  build the package from source.
- **Moved `[tool.ruff]`:** Moved the `[tool.ruff]` configuration higher in the
  file for better organization.
- **Refactored project urls:** Correctly use the `[project.urls]` table, as
  specified by PEP 621.

This change allows developers to choose between using Poetry or other
dependency management and installation tools.  Poetry can still be used for
building and publishing, leveraging its existing tooling.
Resolves Poetry dependency resolver error by explicitly setting Python version
upper bound to prevent conflicts with textual-dev requirements.
Copy link
Collaborator

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

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

This will have to be a "taking your word for it" change. Thanks!

pyproject.toml Outdated
Comment on lines 28 to 37
include = [
"src/textual/py.typed",
{ path = "docs/examples", format = "sdist" },
{ path = "tests", format = "sdist" },
# The reason for the slightly convoluted path specification here is that
# poetry populates the exclude list with the content of .gitignore, and
# it also seems like exclude trumps include. So here we specify that we
# want to package up the content of the docs-offline directory in a way
# that works around that.
{ path = "docs-offline/**/*", format = "sdist" },
Copy link
Contributor

Choose a reason for hiding this comment

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

Should docs and tests be included in sdist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TomJGooding Thank you for catching this oversight. I've restored the include directives in the latest commit to ensure docs and tests are properly packaged in the sdist, maintaining the project's original behavior.

@zarch
Copy link
Contributor Author

zarch commented Feb 23, 2025

This will have to be a "taking your word for it" change. Thanks!

I appreciate your trust, but I'd strongly encourage testing this change thoroughly first. While it works in my local environment with both Poetry and uv, PEP 621 changes can have subtle implications. Since this affects the project's core configuration, it would be best to verify it works as expected in your environment and CI pipelines before merging.
I opened this PR to potentially benefit the wider community, but I value project stability over speed of adoption. Happy to help with testing or make adjustments if needed.

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.

3 participants