Apply more assorted Pyugrade suggestions #4125

DimitriPapadopoulos · 2023-11-20T21:51:49Z

Summary of changes

Apply more suggestions from pyupgrade.

Pull Request Checklist

Changes have tests
News fragment added in newsfragments/.
(See documentation for details)

abravalheri

Thank you very much @DimitriPapadopoulos.

I will not merge this immediately because I am waiting to see if any other bugfixes are necessary for v69, but I think we can merge the PR soon in the future.

DimitriPapadopoulos · 2023-11-21T15:55:33Z

Thank you for looking into this.

Would you agree to add the pyupgrade linter once I have finished fixing the issues reported by pyupgrade?

abravalheri · 2023-11-21T18:17:28Z

Would you agree to add the pyupgrade linter once I have finished fixing the issues reported by pyupgrade?

If @jaraco is OK with that, I think this change would be fine (we will need to prevent/ignore changes in the setuptools._distutils and setuptools._vendor paths, though).

DimitriPapadopoulos · 2023-11-21T18:30:25Z

By the way, I don't know how you feel about ruff, but it lets you disable pyupgrade rules, while the original pyupgrade is rather opinionated and does not let you disable any of its rules.

abravalheri · 2023-11-21T19:11:13Z

I think we are already using pytest-ruff for some stuff... But I guess pytest-ruff requires a more verbose configuration in order to do the pyupgrade stuff (it would be nice if they implement profiles like isort did once...).

Avasam · 2023-11-22T00:18:44Z

A sample ruff.toml would look like:

extend-select = [
  "UP", # pyupgrade
]
extend-ignore = [
  "UP015",  # redundant-open-modes, explicit is prefered
]
extend-exclude = [
  # Vendored
  "**/_vendor",
  "setuptools/_distutils",
]

DimitriPapadopoulos · 2023-11-22T00:29:26Z

Doesn't pytest-ruff search its config in pyproject.toml like ruff, in addition to ruff.toml? I am asking because we could avoid an extra config file.

Avasam · 2023-11-22T00:48:19Z

Doesn't pytest-ruff search its config in pyproject.toml like ruff, in addition to ruff.toml? I am asking because we could avoid an extra config file.

@jaraco 's previously written opinion on this, which I think will apply just the same to Ruff: #3979 (review)

I do wish to retain the mypy settings in mypy.ini for two reasons:

The skeleton for this project is maintained upstream, which defines the mypy settings in its own file. Moving the settings to a different file makes it difficult to compare and manage the differences with settings inherited from upstream.

Piling all project settings in pyproject.toml is a bad idea in general. I've been meaning to write a blog post about it, but the tl;dr is that because there's no ordering or namespacing, the settings lack structure and consistency (compared with mypy which always appears after docs/ and before newsfragments/ and is clearly distinct from towncrier or tox or pytest settings.

DimitriPapadopoulos · 2023-11-22T09:55:37Z

I have added a ruff.toml file in 7e986ee.

I have disabled warnings about string interpolation for now, because the changes are really pervasive. Also, I find f-strings might be less readable when string interpolation involves a complex expression instead of a mere variable. In such cases, I feel keeping .format() or adding a temporary variable to store the result of the complex expression is preferable. Do you want me to proceed nevertheless?

abravalheri · 2023-11-22T10:00:15Z

Let's wait to see if Jason is Ok with the approach. Worst case scenario we can remove the ruff.toml file and merge the PR as it is.

jaraco · 2023-11-22T14:08:12Z

Don’t let me be the blocker here. Normally, I’d like changes like these to be considered at jaraco/skeleton so they can apply to all projects and be kept in sync. The specific effects on this codebase, and configuration specific to this codebase, however, would need to be submitted here. So let’s move forward with this change here and see what implications it has for the general approach. I’m pleased to see the use of Ruff over pyupgrade. Id also be interested to see Ruff replace black and implement other common formatting/linking like import sorts.

abravalheri · 2023-11-22T17:42:37Z

@DimitriPapadopoulos, let's merge this PR as it is then (but I will not cutting a release right now, just add the changes to main, I hope that is OK).
I will just wait another day to see if we don't have any other backlash on v69 before doing the merge.

I have disabled warnings about string interpolation for now, because the changes are really pervasive. Also, I find f-strings might be less readable when string interpolation involves a complex expression instead of a mere variable. In such cases, I feel keeping .format() or adding a temporary variable to store the result of the complex expression is preferable. Do you want me to proceed nevertheless?

If that would allow us to reduce the configuration size for Ruff, I like the idea of temporary variable. I think it is worth to be as streamline as possible. But let's do that in a follow up PR...

DimitriPapadopoulos · 2023-11-23T10:08:27Z

I’m pleased to see the use of Ruff over pyupgrade. Id also be interested to see Ruff replace black and implement other common formatting/linking like import sorts.

I have started working on this in jaraco/skeleton#99.

abravalheri · 2023-11-23T17:11:50Z

I will just wait another day to see if we don't have any other backlash on v69 before doing the merge.

Postponed until we figure out what happened on #4136

DimitriPapadopoulos · 2023-11-23T17:12:07Z

@abravalheri I have noticed that jaraco/skeleton uses the recent skip-string-normalization black option:

jaraco/skeleton/pyproject.toml

[tool.black]
skip-string-normalization = true

Should I instruct black to leave quotes as they are, as in jaraco/skeleton? On the other hand, this project has already been black'ified with double quotes everywhere.

abravalheri · 2023-11-23T17:28:27Z

I think that setuptools should target to have ruff.toml as close as possible to skeleton, so that it can be merged easily.

abravalheri · 2024-01-04T10:29:49Z

ruff.toml

@@ -1,4 +1,3 @@
-[lint]


Should we just leave the [lint] section header so it differs less from the skeleton configuration?

Good catch. I should rather fix the skeleton configuration as ignore is part of the Top-level Ruff Settings:
jaraco/skeleton#106

I think that the ruff docs are a bit outdated, the main example uses the [lint] section, and the detailed reference puts them the the global scope... Which is a bit confusing :P

Henry once told me that in the past Ruff docs would use the global config but now they are going towards [lint]...
In this PR we can see the author referencing the new lint sections:

Breaking up "Configuration" into "Configuring Ruff" (for generic configuration), and new Linter- and Formatter-specific sections.

Updating all example configurations to use [tool.ruff.lint] and [tool.ruff.format].

So that sounds like it is the most up-to-date thing to do (at least until the next release 😅)

They are going towards lint for all options that are related to lint, that's for sure. However, ignore applies to both lint and format. At least that's my understanding.

See jaraco/skeleton#106 (comment).

Perfect, we got a definitive answer from the maintainers: astral-sh/ruff#8297 (reply in thread)

I'll put back [lint].

Done. But note this requires to duplicate extend-exclude, both under [lint] and [format].

Thank you very much @DimitriPapadopoulos.

Yeah, not ideal. But I guess it is good to go with Ruff maintainer's recommendation.

This is a suggestion from pyupgrade: https://github.com/asottile/pyupgrade#yield--yield-from

In Python 3, io.open() is an alias for the builtin open() function. https://docs.python.org/3/library/io.html#io.open This is a suggestion from pyupgrade: https://github.com/asottile/pyupgrade#open-alias

See PEP 3135: https://peps.python.org/pep-3135/ This is a suggestion from pyupgrade: https://github.com/asottile/pyupgrade#super-calls

Fixed by running `ruff --select UP018 --fix .`: UP018 [*] Unnecessary `str` call (rewrite as a literal) Original suggestions from pyupgrade: https://github.com/asottile/pyupgrade#forced-strnative-literals

This is an alias for the built-in OSError exception: https://docs.python.org/3/library/os.html#os.error Fixed by running `ruff --select UP024 --fix .`: UP024 [*] Replace aliased errors with `OSError`

In Python ≥ 3.7, `capture_output` can be used instead of `stdout=PIPE` / `stderr=PIPE`. Fixed by running `ruff --select UP022 --fix --unsafe-fixes .`: UP022 Sending `stdout` and `stderr` to `PIPE` is deprecated, use `capture_output`

Fixed by running `ruff --select UP034 --fix .`: UP034 [*] Avoid extraneous parentheses

In Python 3, the source encodign is implict, UTF-8 by default. This is a suggestion from pyupgrade: https://github.com/asottile/pyupgrade#-coding--comment Fixed by running `ruff --select UP009 --fix .`: UP009 [*] UTF-8 encoding declaration is unnecessary

This is a suggestion from pyupgrade: https://github.com/asottile/pyupgrade#encode-to-bytes-literals Fixed by running `ruff --select UP012 --fix .`: UP012 [*] Unnecessary call to `encode` as UTF-8

Fixed by running `ruff --select UP027 --fix .`: UP027 [*] Replace unpacked list comprehension with a generator expression

abravalheri · 2024-01-05T14:52:57Z

I just squashed the first and last commit together because I want to avoid as much as I can future git conflicts.

ruff.toml

abravalheri · 2024-01-05T20:21:45Z

Thank you very much for the contribution!

abravalheri approved these changes Nov 21, 2023

View reviewed changes

DimitriPapadopoulos force-pushed the pyupgrade branch 3 times, most recently from 1ead630 to b2bb9f7 Compare November 22, 2023 01:19

DimitriPapadopoulos mentioned this pull request Nov 23, 2023

Use the ruff formatter, add more ruff linter rules jaraco/skeleton#99

Merged

This was referenced Dec 9, 2023

black/isort → ruff format Blosc/python-blosc2#151

Closed

black → ruff format codespell-project/codespell#3242

Merged

DimitriPapadopoulos force-pushed the pyupgrade branch from b2bb9f7 to 2fc03cb Compare January 3, 2024 22:55

abravalheri reviewed Jan 4, 2024

View reviewed changes

abravalheri mentioned this pull request Jan 4, 2024

ruff: lint.ignore → ignore jaraco/skeleton#106

Closed

DimitriPapadopoulos added 5 commits January 5, 2024 14:50

Add pyupgrade rules to ruff config

0f08db8

"yield from", instead of "yield" in a loop

ac3bf62

This is a suggestion from pyupgrade: https://github.com/asottile/pyupgrade#yield--yield-from

io.open() → open()

af41360

In Python 3, io.open() is an alias for the builtin open() function. https://docs.python.org/3/library/io.html#io.open This is a suggestion from pyupgrade: https://github.com/asottile/pyupgrade#open-alias

Simplify super() calls

bba79a0

See PEP 3135: https://peps.python.org/pep-3135/ This is a suggestion from pyupgrade: https://github.com/asottile/pyupgrade#super-calls

Don't cast string literals with str()

1ddffe4

Fixed by running `ruff --select UP018 --fix .`: UP018 [*] Unnecessary `str` call (rewrite as a literal) Original suggestions from pyupgrade: https://github.com/asottile/pyupgrade#forced-strnative-literals

DimitriPapadopoulos added 6 commits January 5, 2024 14:50

os.error → OSError

8da1688

This is an alias for the built-in OSError exception: https://docs.python.org/3/library/os.html#os.error Fixed by running `ruff --select UP024 --fix .`: UP024 [*] Replace aliased errors with `OSError`

Use capture_output

a628dc5

In Python ≥ 3.7, `capture_output` can be used instead of `stdout=PIPE` / `stderr=PIPE`. Fixed by running `ruff --select UP022 --fix --unsafe-fixes .`: UP022 Sending `stdout` and `stderr` to `PIPE` is deprecated, use `capture_output`

Remove extraneous pair of prentheses

b5f07c9

Fixed by running `ruff --select UP034 --fix .`: UP034 [*] Avoid extraneous parentheses

Use byte literals instead of .encode()

5ba5c2e

This is a suggestion from pyupgrade: https://github.com/asottile/pyupgrade#encode-to-bytes-literals Fixed by running `ruff --select UP012 --fix .`: UP012 [*] Unnecessary call to `encode` as UTF-8

Use generator instead of list

a142991

Fixed by running `ruff --select UP027 --fix .`: UP027 [*] Replace unpacked list comprehension with a generator expression

abravalheri force-pushed the pyupgrade branch from 27ffe50 to a142991 Compare January 5, 2024 14:51

abravalheri reviewed Jan 5, 2024

View reviewed changes

ruff.toml Show resolved Hide resolved

ruff.toml Show resolved Hide resolved

Ignore path that are automatically generated

07774d9

abravalheri merged commit d3048fc into pypa:main Jan 5, 2024
21 of 23 checks passed

DimitriPapadopoulos deleted the pyupgrade branch January 5, 2024 20:23

DimitriPapadopoulos mentioned this pull request May 22, 2024

Enforce ruff/pyupgrade rules (UP) #4379

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply more assorted Pyugrade suggestions #4125

Apply more assorted Pyugrade suggestions #4125

DimitriPapadopoulos commented Nov 20, 2023

abravalheri left a comment

DimitriPapadopoulos commented Nov 21, 2023

abravalheri commented Nov 21, 2023

DimitriPapadopoulos commented Nov 21, 2023

abravalheri commented Nov 21, 2023

Avasam commented Nov 22, 2023 •

edited

Loading

DimitriPapadopoulos commented Nov 22, 2023 •

edited

Loading

Avasam commented Nov 22, 2023

DimitriPapadopoulos commented Nov 22, 2023

abravalheri commented Nov 22, 2023

jaraco commented Nov 22, 2023 via email

abravalheri commented Nov 22, 2023

DimitriPapadopoulos commented Nov 23, 2023

abravalheri commented Nov 23, 2023

DimitriPapadopoulos commented Nov 23, 2023 •

edited

Loading

abravalheri commented Nov 23, 2023 •

edited

Loading

abravalheri Jan 4, 2024 •

edited

Loading

DimitriPapadopoulos Jan 4, 2024 •

edited

Loading

abravalheri Jan 4, 2024 •

edited

Loading

DimitriPapadopoulos Jan 4, 2024 •

edited

Loading

abravalheri Jan 4, 2024

DimitriPapadopoulos Jan 4, 2024

DimitriPapadopoulos Jan 4, 2024

abravalheri Jan 5, 2024

abravalheri commented Jan 5, 2024 •

edited

Loading

abravalheri commented Jan 5, 2024

Apply more assorted Pyugrade suggestions #4125

Apply more assorted Pyugrade suggestions #4125

Conversation

DimitriPapadopoulos commented Nov 20, 2023

Summary of changes

Pull Request Checklist

abravalheri left a comment

Choose a reason for hiding this comment

DimitriPapadopoulos commented Nov 21, 2023

abravalheri commented Nov 21, 2023

DimitriPapadopoulos commented Nov 21, 2023

abravalheri commented Nov 21, 2023

Avasam commented Nov 22, 2023 • edited Loading

DimitriPapadopoulos commented Nov 22, 2023 • edited Loading

Avasam commented Nov 22, 2023

DimitriPapadopoulos commented Nov 22, 2023

abravalheri commented Nov 22, 2023

jaraco commented Nov 22, 2023 via email

abravalheri commented Nov 22, 2023

DimitriPapadopoulos commented Nov 23, 2023

abravalheri commented Nov 23, 2023

DimitriPapadopoulos commented Nov 23, 2023 • edited Loading

abravalheri commented Nov 23, 2023 • edited Loading

abravalheri Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

DimitriPapadopoulos Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

abravalheri Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

DimitriPapadopoulos Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

abravalheri Jan 4, 2024

Choose a reason for hiding this comment

DimitriPapadopoulos Jan 4, 2024

Choose a reason for hiding this comment

DimitriPapadopoulos Jan 4, 2024

Choose a reason for hiding this comment

abravalheri Jan 5, 2024

Choose a reason for hiding this comment

abravalheri commented Jan 5, 2024 • edited Loading

abravalheri commented Jan 5, 2024

Avasam commented Nov 22, 2023 •

edited

Loading

DimitriPapadopoulos commented Nov 22, 2023 •

edited

Loading

DimitriPapadopoulos commented Nov 23, 2023 •

edited

Loading

abravalheri commented Nov 23, 2023 •

edited

Loading

abravalheri Jan 4, 2024 •

edited

Loading

DimitriPapadopoulos Jan 4, 2024 •

edited

Loading

abravalheri Jan 4, 2024 •

edited

Loading

DimitriPapadopoulos Jan 4, 2024 •

edited

Loading

abravalheri commented Jan 5, 2024 •

edited

Loading