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

Add support for simple generic type variables to UP040 #6314

Merged
merged 9 commits into from
Aug 7, 2023

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Aug 3, 2023

Extends #6289 to support moving type variable usage in type aliases to use PEP-695.

Does not remove the possibly unused type variable declaration. Presumably this is handled by other rules, but is not working for me. We cannot remove module level declarations safely.

Does not handle type variables with bounds or variance declarations yet.

Part of #4617

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.01      8.3±0.13ms     4.9 MB/sec    1.00      8.2±0.17ms     5.0 MB/sec
formatter/numpy/ctypeslib.py               1.01  1644.6±44.20µs    10.1 MB/sec    1.00  1625.4±30.34µs    10.2 MB/sec
formatter/numpy/globals.py                 1.02    186.0±6.90µs    15.9 MB/sec    1.00    181.8±3.22µs    16.2 MB/sec
formatter/pydantic/types.py                1.01      3.5±0.06ms     7.3 MB/sec    1.00      3.4±0.07ms     7.4 MB/sec
linter/all-rules/large/dataset.py          1.00     10.1±0.04ms     4.0 MB/sec    1.00     10.1±0.02ms     4.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      2.7±0.02ms     6.2 MB/sec    1.01      2.7±0.01ms     6.2 MB/sec
linter/all-rules/numpy/globals.py          1.00    377.2±1.39µs     7.8 MB/sec    1.01    380.7±1.97µs     7.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      4.6±0.04ms     5.5 MB/sec    1.00      4.6±0.03ms     5.5 MB/sec
linter/default-rules/large/dataset.py      1.00      5.2±0.01ms     7.8 MB/sec    1.00      5.2±0.01ms     7.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1123.8±2.80µs    14.8 MB/sec    1.01   1132.3±4.46µs    14.7 MB/sec
linter/default-rules/numpy/globals.py      1.00    127.7±1.28µs    23.1 MB/sec    1.01    128.9±2.59µs    22.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.3±0.00ms    10.9 MB/sec    1.01      2.3±0.01ms    10.9 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.05     10.3±0.07ms     3.9 MB/sec    1.00      9.8±0.08ms     4.1 MB/sec
formatter/numpy/ctypeslib.py               1.05  1952.7±26.25µs     8.5 MB/sec    1.00  1863.1±18.09µs     8.9 MB/sec
formatter/numpy/globals.py                 1.03    197.0±3.39µs    15.0 MB/sec    1.00    191.4±3.45µs    15.4 MB/sec
formatter/pydantic/types.py                1.05      4.4±0.03ms     5.8 MB/sec    1.00      4.2±0.10ms     6.1 MB/sec
linter/all-rules/large/dataset.py          1.00     12.6±0.14ms     3.2 MB/sec    1.00     12.6±0.19ms     3.2 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.03ms     4.7 MB/sec    1.00      3.5±0.02ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.01    366.8±5.68µs     8.0 MB/sec    1.00    362.5±7.92µs     8.1 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.8±0.04ms     4.4 MB/sec    1.01      5.9±0.04ms     4.3 MB/sec
linter/default-rules/large/dataset.py      1.00      6.7±0.09ms     6.1 MB/sec    1.02      6.8±0.04ms     6.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1384.0±10.99µs    12.0 MB/sec    1.01  1391.7±15.61µs    12.0 MB/sec
linter/default-rules/numpy/globals.py      1.00    139.9±1.84µs    21.1 MB/sec    1.01    141.2±2.18µs    20.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.0±0.07ms     8.5 MB/sec    1.01      3.0±0.02ms     8.4 MB/sec

@charliermarsh
Copy link
Member

Does not remove the possibly unused type variable declaration. Presumably this is handled by other rules, but is not working for me.

We don't remove unused variables in the module scope, since they could be imported from other modules (i.e., they are part of the module's public API). (We only remove unused variables within function scopes.)

@zanieb
Copy link
Member Author

zanieb commented Aug 3, 2023

We don't remove unused variables in the module scope, since they could be imported from other modules (i.e., they are part of the module's public API). (We only remove unused variables within function scopes.)

Ah that makes sense. We're pretty unlikely to ever remove these then. I don't think we should either, I've definitely imported them :D

@charliermarsh charliermarsh added the fixes Related to suggested fixes for violations label Aug 4, 2023
fn visit_expr(&mut self, expr: &'a Expr) {
match expr {
Expr::Name(name) if name.ctx.is_load() => {
let Some(Stmt::Assign(StmtAssign { value, .. })) =
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to match AnnAssign too? No, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. At least, Pyright complains about

from typing import TypeVar

T: TypeVar = TypeVar("T")

@zanieb zanieb merged commit 90c9aa2 into main Aug 7, 2023
17 checks passed
@zanieb zanieb deleted the rule/type-alias-generic branch August 7, 2023 21:22
durumu pushed a commit to durumu/ruff that referenced this pull request Aug 12, 2023
Extends astral-sh#6289 to support moving type variable usage in type aliases to
use PEP-695.

Does not remove the possibly unused type variable declaration.
Presumably this is handled by other rules, but is not working for me.

Does not handle type variables with bounds or variance declarations yet.

Part of astral-sh#4617
charliermarsh pushed a commit that referenced this pull request Sep 15, 2023
… type variables to UP040 (#6749)

## Summary

Extends UP040 to support moving type variables with
bounds/constraints/variance that are used in type aliases to use PEP-695
syntax.

Part of #4617.

## Test Plan

The existing tests added by #6314 already cover the relevant cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants