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

FA102 errors eventhough requires-python in pyproject.toml is ">=3.12" #10299

Open
SimonMarynissen opened this issue Mar 8, 2024 · 14 comments
Open
Labels
bug Something isn't working configuration Related to settings and configuration

Comments

@SimonMarynissen
Copy link

I have the following (standard) directory layout:

├── src
│   └── packagename
│       └── ...
├── pyproject.toml
└── ruff.toml

Part from my pyproject.toml:

[project]
name = "packagename"
requires-python = ">=3.12"

My complete ruff.toml:

src = ["src"]

[format]
skip-magic-trailing-comma = true
line-ending = "lf"

[lint]
select = ["ALL"]
ignore = ["ANN101", "D203", "D213", "S320"]

Ruff version: 0.3.0, run with the command ruff check in the directory of pyproject.toml.
In the rule FA102, it is mentioned:

This rule respects the target-version setting. For example, if your project targets Python 3.10 and above, adding from future import annotations does not impact your ability to leverage PEP 604-style unions (e.g., to convert Optional[str] to str | None). As such, this rule will only flag such usages if your project targets Python 3.9 or below.

In the documentation of target-version it is mentioned that requires-python in pyproject.toml is preferred over the target-version option in ruff.toml.
If I add target-version = "py312" to my ruff.toml, then there are no FA102 errors.

@charliermarsh
Copy link
Member

I think it's possible that if we find a ruff.toml, we don't even look for the pyproject.toml, since we only use a single configuration file to resolve configuration for the project. So you could fix this (I suspect) by moving your configuration into your pyproject.toml, like:

[project]
name = "packagename"
requires-python = ">=3.12"

[tool.ruff]
src = ["src"]

[tool.ruff.format]
skip-magic-trailing-comma = true
line-ending = "lf"

[tool.ruff.lint]
select = ["ALL"]
ignore = ["ANN101", "D203", "D213", "S320"]

But, we should probably also be respecting the pyproject.toml here.

@charliermarsh charliermarsh added bug Something isn't working configuration Related to settings and configuration labels Mar 8, 2024
@charliermarsh
Copy link
Member

(If possible, can you give that a try to confirm my suspicion?)

@SimonMarynissen
Copy link
Author

That does indeed work, but I would prefer to have a separate ruff.toml.

@MichaReiser
Copy link
Member

@charliermarsh considering that you marked this as a bug. What would the desired resolution order be?

The documentation mentions

Ruff can be configured through a pyproject.toml, ruff.toml, or .ruff.toml file.

which seems like pyproject.toml intentionally has higher precedence than ruff.toml. Changing the resolution order would probably be a breaking change.

Or is the fix that we should disregard any pyproject.toml without any tool.ruff section (but that could lead to other false positives because we might disregard a requires-python configuration). That makes me think that the only "proper" solution is to merge the configurations, but that might lead to other unintended or unexpected behavior.

@charliermarsh
Copy link
Member

Or is the fix that we should disregard any pyproject.toml without any tool.ruff section (but that could lead to other false positives because we might disregard a requires-python configuration).

We already do this IIRC.

@charliermarsh
Copy link
Member

That makes me think that the only "proper" solution is to merge the configurations, but that might lead to other unintended or unexpected behavior.

Yeah fixing this requires that we merge configuration in some way, because the user is inherently asking for information to come from two different files. So we either do something like "when we see a ruff.toml, check for a pyproject.toml in the same directory and use its requires-python", or mark it as unsupported.

@SimonMarynissen
Copy link
Author

If I may add my 2 cents: if the precedence is pyproject.toml > ruff.toml > .ruff.toml, then pyproject.toml is always parsed (if it exists) and if no ruff elements are found, I guess you either get bad defaults or an error (I am not too familiar with ruff), so it shouldn't be big change to look for ruff.toml in that case. That would certainly solve my case and would most likely not break workflows from other ruff users.

@DimitriPapadopoulos
Copy link
Contributor

Reproduced in sphinx-doc/sphinx#12250. Removing target-version = "py39" from .ruff.toml breaks the tests as requires-python = ">=3.9" from pyproject.toml is not taken into account.

I think ruff should take into account requires-python" from pyproject.toml, alternatively fix the documentation:

/// 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"`:

@DimitriPapadopoulos
Copy link
Contributor

Same issue in jaraco/skeleton#119. The situation is even worse here, as requires-python is defined in setup.cfg instead of pyproject.toml.

The rationale for a ruff.toml file separate from pyproject.toml is described here:
pypa/setuptools#3979 (review)

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented Apr 15, 2024

In the documentation, Config file discovery reads:

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.

I was thinking about adding extend = "pyproject.toml" to ruff.toml to pick requires-python from pyproject.toml, but this doesn't make sense as the namespaces and the way the files are parsed are different.

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented Apr 15, 2024

My suggestion would be to:

  1. Keep a single configuration file for ruff itself, that is, disregard the [tool.ruff] subtable in pyproject.toml if files .ruff.toml or ruff.toml exist - or ruff is passed option --config.
  2. Take into account other tables of pyproject.toml that contain keys that impact ruff, typically the [project] table and its requires-python key.
  3. Support keys of interest in both setup.cfg and pyproject.toml. Not sure about precedence in the ill-formed case where requires-python would be defined in both setup.cfg and pyproject.toml.
  4. Keep the existing precedence if pyproject.toml or setup.cfg define requires-python and the ruff config file defines target-version simultaneously: the ruff config file and target-version take precedence.

DimitriPapadopoulos added a commit to DimitriPapadopoulos/skeleton that referenced this issue Apr 15, 2024
If `ruff.toml` exists, the `requires-python` key is currently not taken
into account - not in `pyproject.toml` and even less in `setup.cfg`.

See:
astral-sh/ruff#10299
DimitriPapadopoulos added a commit to DimitriPapadopoulos/skeleton that referenced this issue Apr 15, 2024
If `ruff.toml` exists, the `requires-python` key is currently not taken
into account - not in `pyproject.toml` and even less in `setup.cfg`.

See:
astral-sh/ruff#10299
@kubotty
Copy link

kubotty commented Oct 28, 2024

Hi, I encountered the same issue and I found this solution.
This is what is mentioned:

I was thinking about adding extend = "pyproject.toml" to ruff.toml to pick requires-python from pyproject.toml, but this doesn't make sense as the namespaces and the way the files are parsed are different.

However, I tried writing extend = "./pyproject.toml" in ruff.toml and it worked.

@jaraco
Copy link
Contributor

jaraco commented Oct 28, 2024

I'd like to make the case that ruff should not be relying on declared configuration and should instead rely on the core metadata for the package. Although pyproject.toml is a standard for a source distribution to declare metadata, it's not (and should not) be required that the package's metadata be declared through pyproject.toml. As an extreme example, the coherent build system doesn't require any pyproject.toml and instead derives all of the metadata (including the Requires-Python declaration) from environmental state, specifically the tags on the CPython repository in GitHub.

For this reason, the supported/required Python versions for a Coherent project or any other build system that dynamically or programmatically generates that metadata field won't have a declaration in the pyproject.toml nor ruff.toml.

There are other build backends that produce "dynamic" values for various metadata fields, so addressing just the Requires-Python field will leave those fields unaddressed.

There's clear value for projects in not having to statically manifest all of their metadata in source code (and maintain that state, possibly shared across or even diverging within many projects).

My recommendation would be for Ruff to generate the package metadata through the PEP 517 prepare_metadata_for_build_wheel hook and then read the Requires-Python field.

I know this approach can be potentially expensive (and for Coherent projects is currently very expensive, but that's largely the backend's problem), so it may be necessary to cache the value (or the metadata) for extended periods.

This approach would have added benefits that other core metadata can be resolved by ruff, even when it's dynamic.

Such an approach would sidestep workarounds like extend = "pyproject.toml" or duplicating the metadata in two files and would honor the essential metadata of the package.

jaraco added a commit to jaraco/skeleton that referenced this issue Oct 28, 2024
@Avasam
Copy link

Avasam commented Oct 30, 2024

As mentioned in pypa/setuptools#4718 (comment), it seems include = ["pyproject.toml"] doesn't really work as a true workaround, it just hides version-related errors, even if requires-python is set in the pyproject.toml, even if you explicitly pass the flag by CLI: ruff check --select=UP035 --target-version=py39

Edit: because it should be extend = "pyproject.toml", not include...

jaraco added a commit to jaraco/skeleton that referenced this issue Nov 2, 2024
jaraco added a commit to jaraco/skeleton that referenced this issue Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working configuration Related to settings and configuration
Projects
None yet
Development

No branches or pull requests

7 participants