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

improve error message of PERF401 for extends #11067

Closed
trim21 opened this issue Apr 21, 2024 · 6 comments
Closed

improve error message of PERF401 for extends #11067

trim21 opened this issue Apr 21, 2024 · 6 comments

Comments

@trim21
Copy link

trim21 commented Apr 21, 2024

import os
from pathlib import Path
from typing import Any


directory: Any = []

files: list[Path] = []
for p in directory:
    if p.is_file():
        files.append(p)
    else:
        for dir, _, _files in os.walk(p):
            for _f in _files:
                files.append(Path(dir).joinpath(_f)) # PERF401 Use a list comprehension to create a transformed list

https://play.ruff.rs/cde69c3f-1d15-4089-bb62-5de79566898a

ruff version 0.4.1

(Is this even possible to re-write with list comprehension?)

@JonathanPlasse
Copy link
Contributor

You can rewrite it like this. Only the inner for loops need to be rewritten.

import os
from pathlib import Path
from typing import Any


directory: Any = []

files: list[Path] = []
for p in directory:
    if p.is_file():
        files.append(p)
    else:
        files.extend(
            Path(dir).joinpath(_f) for dir, _, _files in os.walk(p) for _f in _files
        )

@trim21
Copy link
Author

trim21 commented Apr 21, 2024

You can rewrite it like this. Only the inner for loops need to be rewritten.

import os
from pathlib import Path
from typing import Any


directory: Any = []

files: list[Path] = []
for p in directory:
    if p.is_file():
        files.append(p)
    else:
        files.extend(
            Path(dir).joinpath(_f) for dir, _, _files in os.walk(p) for _f in _files
        )

so the problem is error message not very clear…

@JonathanPlasse
Copy link
Contributor

Indeed

@trim21
Copy link
Author

trim21 commented Apr 21, 2024

I'm not a fan of nested loop in list comprehension repr, I would just disable this rule and leave this issue open...

@JonathanPlasse
Copy link
Contributor

The documentation does precise that .extend can be used for existing lists.

@trim21 trim21 changed the title false positive of PERF401 for complex append improve error message of PERF401 for extends Apr 21, 2024
@charliermarsh
Copy link
Member

I think this is actually okay because it is included in the documentation, and it would be hard to know whether or not to recommend this in the error message itself.

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

No branches or pull requests

3 participants