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

Update pyproject-toml to support PEP 639 #13902

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update pyproject-toml to support PEP 639 #13902

wants to merge 1 commit into from

Conversation

konstin
Copy link
Member

@konstin konstin commented Oct 24, 2024

PEP 639 is provisionally accepted, we were previously shipping definitions from an earlier draft.

We don't validate all that's in the PEP (license vs. license-files compatibility, SPDX expressions), only the overall structure of pyproject.toml.

Updates pep440_rs in the process.

Fixes #13869

@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
Copy link
Member Author

Choose a reason for hiding this comment

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

This now became a valid example

---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
pyproject.toml:10:16: RUF200 Failed to parse pyproject.toml: Expected package name starting with an alphanumeric character, found '.'
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently, we only emit a single error when various things are broken

@@ -1,13 +1,13 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
pyproject.toml:6:84: RUF200 Failed to parse pyproject.toml: invalid type: integer `1`, expected a string
pyproject.toml:5:11: RUF200 Failed to parse pyproject.toml: a table with 'name' and/or 'email' keys
Copy link
Member Author

Choose a reason for hiding this comment

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

The error message isn't ideal but the span should sufficiently prompt users to fix their authors table

@konstin konstin added the rule Implementing or modifying a lint rule label Oct 24, 2024
Copy link

codspeed-hq bot commented Oct 24, 2024

CodSpeed Performance Report

Merging #13902 will improve performances by 5.21%

Comparing konsti/pep639 (6a54d3b) with main (70bdde4)

Summary

⚡ 1 improvements
✅ 31 untouched benchmarks

Benchmarks breakdown

Benchmark main konsti/pep639 Change
red_knot_check_file[incremental] 4.3 ms 4.1 ms +5.21%

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Thanks. This looks good. Unfortunately, we need to find a solution for wasm :(

Copy link
Contributor

github-actions bot commented Oct 24, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+2 -0 violations, +0 -0 fixes in 2 projects; 52 projects unchanged)

apache/airflow (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ clients/python/pyproject.toml:27:1: RUF200 Failed to parse pyproject.toml: invalid type: map, expected a sequence
+ pyproject.toml:58:1: RUF200 Failed to parse pyproject.toml: invalid type: map, expected a sequence

pandas-dev/pandas (+0 -0 violations, +0 -0 fixes)


Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF200 2 2 0 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+2 -0 violations, +0 -0 fixes in 1 projects; 53 projects unchanged)

apache/airflow (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ clients/python/pyproject.toml:27:1: RUF200 Failed to parse pyproject.toml: invalid type: map, expected a sequence
+ pyproject.toml:58:1: RUF200 Failed to parse pyproject.toml: invalid type: map, expected a sequence

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF200 2 2 0 0 0

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser
Copy link
Member

I went ahead and disabled the pyproject_toml linting for wasm to unblock this PR. That's why I request review from someone else.

@dhruvmanila
Copy link
Member

Looking at the ecosystem changes, it seems that we'll now create diagnostic for the old definition related to license-files: https://github.com/apache/airflow/blob/476e77d9aba492a2159b256bc7a4aa86e560dee5/clients/python/pyproject.toml#L27. Do we want to maintain backwards compatibility?

@konstin
Copy link
Member Author

konstin commented Oct 25, 2024

I don't think so, that syntax was only part of a draft and never of any (provisionally) accepted PEP. It makes sense to lint against it to migrate the early adopters to the final version (and i'm expecting that hatch will migrate, too).

@MichaReiser
Copy link
Member

Let's make this a breaking change.

@MichaReiser MichaReiser added the breaking Breaking API change label Oct 26, 2024
@MichaReiser MichaReiser added this to the v0.8 milestone Oct 26, 2024
@MichaReiser
Copy link
Member

@konstin added wasm upstream. We can roll-back my rule exclusion once the new version is out. See PyO3/pyproject-toml-rs@2ed4df2

PEP 639 is provisionally accepted, we were previously shipping definitions from an earlier draft.

We don't validate all that's in the PEP (`license` vs. `license-files` compatibility, SPDX expressions), only the overall structure of pyproject.toml.

Updates pep440_rs in the process.

Fixes #13869
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking API change rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for PEP 639 in RUF200
3 participants