jaraco / skeleton

A generic project skeleton for Python projects.
http://blog.jaraco.com/skeleton/
MIT License
123 stars 36 forks source link

`target-version` in `ruff.toml` #119

Closed DimitriPapadopoulos closed 1 month ago

DimitriPapadopoulos commented 7 months ago

According to the target-version documentation, ruff reads the target Python version from project.requires-python in pyproject.toml:

If you're already using a pyproject.toml file, we recommend project.requires-python instead, as it's based on Python packaging standards, and will be respected by other tools. For example, Ruff treats the following as identical to target-version = "py38":

[project]
requires-python = ">=3.8"

Since this skeleton defines requires-python in setup.cfg, not in pyproject.toml, it is not picked up. Therefore, we must redefine the minimal Python version as target-version in ruff.toml. This brings the question, are you planning on moving from setup.cfg to pyproject.toml? Alternatively, would it be possible to move the requires-python from setup.cfg to pyproject.toml?

Additionally, ruff does not read project.requires-python from pyproject.toml if a ruff.toml file exists. It is unclear at this point whether this is a bug or a feature: https://github.com/astral-sh/ruff/issues/10299

bswck commented 7 months ago

In the future, Coherent OSS aims to deliver a "zero-config" project with configuration composable from various files in a special subdirectory (for configuration files only) in every downstream project repository. pyproject.toml is a monolith configuration file and, judging by https://github.com/pypa/setuptools/issues/4025#issuecomment-1704339687, I'd assume that pyproject.toml would be the last resort for configuring anything else than the build backend—knowing the general approach of skeleton to focus on improving composability and strong cohesion (which helps prevent merge conflicts of "monoliths" like pyproject.toml when merging skeleton).

Since Ruff provides the convenience of automatic reading of pyproject.toml, it might be a good suggestion to the Ruff team to also support other formats, including setup.cfg, in a similar way that coverage.py does.

I'm however only speaking for myself here, it's all up to @jaraco here.

DimitriPapadopoulos commented 7 months ago

@bswck I agree with you, perhaps you should add your comment in https://github.com/astral-sh/ruff/issues/10299.

Note that the problem is two-fold:

  1. The authors of ruff seem to intentionally support a single config file. From Config file discovery in their documentation:

    Unlike ESLint, Ruff does not merge settings across configuration files; instead, the "closest" configuration file is used, and any parent configuration files are ignored. In lieu of this implicit cascade, Ruff supports an extend field, which allows you to inherit the settings from another config file, like so:

    pyproject.toml       ruff.toml

    # Extend the `ruff.toml` file in the parent directory...
    extend = "../ruff.toml"
    
    # ...but use a different line length.
    line-length = 100

    All of the above rules apply equivalently to pyproject.toml, ruff.toml, and .ruff.toml files. If Ruff detects multiple configuration files in the same directory, the .ruff.toml file will take precedence over the ruff.toml file, and the ruff.toml file will take precedence over the pyproject.toml file.

  2. Only pyproject.toml is taken into account, not setup.cfg (and setup.py is even less in the picture of course).
jaraco commented 7 months ago

First and foremost, thanks for raising this issue.

I very much want to avoid providing the "minimum supported Python version" in more than one place. I've worked hard to remove it from Trove Classifiers and tox configs and other places. I'd like also to remove it from the GitHub actions if possible, though I don't think that's possible at this time. I'd like not to add yet another redundant indicator to ruff.toml. The canonical indicator is in the Requires-Python field of the project's metadata.

I believe ruff's reliance on any configuration file from other tooling is misplaced. Ideally, ruff should instead load the "Requries-Python" metadata using standards-based tooling. I can see the desire to extract it from pyproject.toml, as that also has a standards basis. I would actually recommend against ruff (or other tooling) pulling from setup.cfg, as pyproject.toml is to be preferred.

The most reliable and correct way for ruff to get the metadata from any PEP 517 backed project irrespective of their configuration is to use that API:

__requires__ = ['build']
import build.util
print(build.util.project_wheel_metadata('.')['Requires-Python'])

Unfortunately, that routine is potentially expensive, as, depending on the backend and the project, it likely requires downloading and installing all of the build dependencies and even building the project into a wheel before extracting the metadata. Some build backends and standards efforts are working on providing more efficient access to the metadata without having to conduct a full build, but the standard only provides for one set of build dependencies, so that aspect may always be heavy.

This brings the question, are you planning on moving from setup.cfg to pyproject.toml? Alternatively, would it be possible to move the requires-python from setup.cfg to pyproject.toml?

The short answer is yes. I'd like to move all projects to pyproject.toml. Although I have avoided moving configuration for non-build tooling, I do expect the pyproject.toml to be the best place to configure declarative project metadata. I just haven't had the time to make the leap. I've filed #121 to track.

DimitriPapadopoulos commented 7 months ago

Understood.

I'll track #121 for progress, but your target also relies on convincing the ruff authors to read project.requires-python from pyproject.toml, no matter if there are .ruff.toml or ruff.toml files around: https://github.com/astral-sh/ruff/issues/10299#issuecomment-2056215019.

jaraco commented 7 months ago

I think I can make a good case. I'll plan to do that.

jaraco commented 1 month ago

In https://github.com/astral-sh/ruff/issues/10299#issuecomment-2441997018, I made the case for reading the core metadata instead of reading the declared metadata.

In that same issue, I noticed that some have indicated that adding an extend = "pyproject.toml" declaration in ruff.toml may be a suitable workaround for some projects. I believe that workaround might be suitable for skeleton-based projects. Let's try it.

jaraco commented 1 month ago

I realized I don't have a good way to test. I've confirmed that adding extend = "foo.toml" causes failure, and extend = "pyproject.toml" does not fail, so maybe that's good enough evidence we can try it.