Enable Ruff flake8-use-pathlib (PTH) #13795
Conversation
…-Ruff-flake8-use-pathlib-(PTH)
Avasam
changed the title
| @@ -203,7 +203,7 @@ def allowlists(distribution_name: str) -> list[str]: | |||
|
|
|||
| @functools.cache | |||
| def get_gitignore_spec() -> pathspec.PathSpec: | |||
| with open(".gitignore", encoding="UTF-8") as f: | |||
| with Path(".gitignore").open(encoding="UTF-8") as f: | |||
There was a problem hiding this comment.
I prefer the simple open() in this and similar cases. But it's no deal breaker for me. The rest looks good.
scripts/sync_protobuf/tensorflow.py
Outdated
| with open(path, encoding="utf-8") as file: | ||
| with path.open(encoding="utf-8") as file: | ||
| filedata = file.read() |
There was a problem hiding this comment.
In cases like this I'm neutral about path.open(...), but I think path.read_text() would be an improvement
| @@ -203,7 +203,7 @@ def allowlists(distribution_name: str) -> list[str]: | |||
|
|
|||
| @functools.cache | |||
| def get_gitignore_spec() -> pathspec.PathSpec: | |||
| with open(".gitignore", encoding="UTF-8") as f: | |||
| with Path(".gitignore").open(encoding="UTF-8") as f: | |||
| return pathspec.PathSpec.from_lines("gitwildmatch", f.readlines()) | |||
There was a problem hiding this comment.
Unrelated to your above comment or this PR directly. But I just read pathspec's doc and found this:
There is a specialized class,
pathspec.GitIgnoreSpec, which more closely
implements the behavior of gitignore. This usesGitWildMatchPattern
pattern by default and handles some edge cases differently from the generic
PathSpecclass.GitIgnoreSpeccan be used without specifying the pattern
factory::spec = pathspec.GitIgnoreSpec.from_lines(spec_text.splitlines())
| return pathspec.PathSpec.from_lines("gitwildmatch", f.readlines()) | |
| return pathspec.GitIgnoreSpec.from_lines(f.readlines()) |
There was a problem hiding this comment.
Could also omit the call to .readlines(). GitIgnoreSpec.from_lines takes an iterable of lines, so passing the file i/o object directly would work. Not sure if there's any reason to prefer one way or another, though
There was a problem hiding this comment.
You're completely right! I would assume .readlines() to just be a waste here since you have to both read the file, then iterate over the list again, instead of streaming it once.
PathSpec.from_lines simply does patterns = [pattern_factory(line) for line in lines if line]
Kept this one for last. Closes #13295
Some rules in this group are forcing some refactoring to use
pathlib. In some cases, it's a lot cleaner, in others it's debatable.I unconditionally followed all the rules. Please indicate changes you disagree with/preferred in the old style. I'll revert those and disable the relevant rules.
There's also a few
with readandwith writethat can be further rewritten to be more concise withPath.read_textandPath.write_text