Migrate from `autoflake`, `black`, `isort`, `pyupgrade`, `flake8` and `pydocstyle`, to `ruff` #11901

Pierre-Sassoulas · 2024-01-31T13:37:04Z

Follow-up to #11896, this seems to be blocked by astral-sh/ruff#4670 unless we're ready to accept a little churn. There are other tools that we could remove if we activate more ruff message, but I figured doing one at a time would help with the reviews.

Taken from pytest-dev#11885 that was closed.

nicoddemus · 2024-01-31T13:45:37Z

Thanks @Pierre-Sassoulas!

I don't mind the churn, however I would prefer to see ruff replacing the other hooks in the same PR (specially black and autoflake, as well as whitespace, eol, etc), to avoid having multiples rounds of PRs which change a large portion of the code each.

Before merging, we can group all the automatic changes into a single commit and register that commit in https://github.com/pytest-dev/pytest/blob/main/.git-blame-ignore-revs.

pyproject.toml

…3034261 into account

See https://github.com/pytest-dev/pytest/pull/11901\#discussion_r1473038296

See pytest-dev#11901 (comment)

Pierre-Sassoulas · 2024-01-31T19:27:46Z

I'm not sure it's possible to remove pre-commit's eol / whitespace checks because they act on all files when ruff fmt or black will only act on python files. Also @nicoddemus do we remove black for sure ? There's also blacken-doc it could maybe be replace by something like astral-sh/ruff#8854 option... But I'm not sure it's equivalent because docstring != python code area in rst/md. Thus it would introduce two kind of formatting black for docstring and code example, and ruff for the code itself.

bluetech · 2024-01-31T19:36:28Z

The isort looks good to me!

There's also blacken-doc it could maybe be replace by something like astral-sh/ruff#8854 option... But I'm not sure it's equivalent because docstring != python code area in rst/md. Thus it would introduce two kind of formatting black for docstring and code example, and ruff for the code itself.

From https://docs.astral.sh/ruff/formatter/#docstring-formatting it's also supposed to handle rst and md.

nicoddemus · 2024-01-31T20:49:56Z

.pre-commit-config.yaml

+  rev: "v0.1.15"
+  hooks:
+    - id: ruff
+      args: ["--fix"]
 -   repo: https://github.com/PyCQA/flake8


I think we can remove flake8 too. 👍

nicoddemus · 2024-01-31T20:52:20Z

From https://docs.astral.sh/ruff/formatter/#docstring-formatting it's also supposed to handle rst and md.

That's awesome. Did this work? I didn't notice any docs changing (which might mean it worked or did not even run).

Pierre-Sassoulas · 2024-01-31T20:53:41Z

I rebased and squashed the isort commits together since they are reviewed now. Migrated autoflake / flake8 core and pyupgrade. Still missing black blacken-doc and flake8-typing-imports atm

graingert · 2024-01-31T20:57:06Z

Is flake8-typing-imports needed? ruff has support for flake8-type-checking https://github.com/astral-sh/ruff/pull/2147/files

TYP002 through TYP005 isn't needed on projects only supporting python 3.7+

Pierre-Sassoulas · 2024-01-31T20:57:54Z

Apparently it's not supported yet : astral-sh/ruff#2302 (but there's only one check that is still useful)

Pierre-Sassoulas · 2024-01-31T21:02:03Z

We're disabling a lot of message from flake8-docstrings too.

bluetech · 2024-01-31T21:26:32Z

src/_pytest/_io/pprint.py

@@ -376,7 +376,7 @@ def _pprint_simplenamespace(
        context: Set[int],
        level: int,
    ) -> None:
-        if type(object) is _types.SimpleNamespace:
+        if isinstance(object, _types.SimpleNamespace):


This should stay as is if I understand the comment below correctly.

Thank you for the reviews ! 287c651

I'm thinking let's disable this rule entirely, generally it's a good rule but pytest can do weird low-level things and we usually know what we're doing when we use type(..) is ....

All right, I'll remove the manual fix for type / is instance and rebase everything cleanly.

src/_pytest/setuponly.py

Pierre-Sassoulas · 2024-01-31T22:27:06Z

Added ruff-format for code and doc. It changed some doc that blacken doc missed. I can produce a diff specifically for the doc changes because a lot of things changed overall. I still need to check how many checks are actually used in flake8-docstrings to possibly remove flake8 entirely instead of simply disabling the core message. Also got an answer on the astral / isort thread to reduce churn that I need to test.

bluetech · 2024-02-01T10:16:25Z

Looks like most of the diff is from changing the max line length from 88 to 120. I think we should stick with 88 since that's the established style.

Pierre-Sassoulas · 2024-02-01T10:25:15Z

Removed flake8-docstring, because it's a wrapper around pydocstyle, and pydocstyle explicitly asks to use ruff in PyCQA/pydocstyle#658. Also the configuration given in the ruff thread about isort force-alphabetical-sort-within-sections option seems to work, so I used it.

I'm going to check how to format for 88 chars and not have warning for line longer than that next, thank you the pointer @bluetech .

Should I separate the manual fixes in another merge request ?

Pierre-Sassoulas · 2024-02-01T10:55:26Z

Most of the diff now seems to be related to the newly autofixed rules from pydocstyle. (But it should probably be a good idea to rebase and re-apply the pre-commit fixes from zero)

bluetech · 2024-02-01T11:35:20Z

For the docstrings, we should go with D212 and disable D213, that would fit our current style better.

bluetech · 2024-02-01T11:37:26Z

Should I separate the manual fixes in another merge request ?

I think it's OK to do them here. I'll do a full look over once you're ready.

Pierre-Sassoulas · 2024-02-01T12:10:56Z

Ready for review, I removed flake8 entirely even if we don't have an equivalent of flake8-typing-import (but almost all checks are not useful if we don't support python 3.7 and the only check that is useful will be included in ruff at some point see astral-sh/ruff#2302).

Pierre-Sassoulas · 2024-02-01T12:27:25Z

I need to either add one of the autofix from pydocstyle in the unfixable list so it doesn't break the doctests, or exclude the code example from ruff-format (example_scripts/doctests/main.py, maybe others)

bluetech · 2024-02-01T13:13:57Z

Looks at the docstring changes. Let's set

[tool.ruff.lint.pydocstyle]
convention = "pep257"

this allows to remove the ignores for the conflicting rules D203 and D213 since the convention implies that.

Also I see many changes to one-sentence docstrings

 def _check_if_assertion_pass_impl() -> bool:
     """Check if any plugins implement the pytest_assertion_pass hook
    in order not to generate explanation unnecessarily (might be expensive)."""

into

 def _check_if_assertion_pass_impl() -> bool:
     """Check if any plugins implement the pytest_assertion_pass hook
    in order not to generate explanation unnecessarily (might be expensive).
    """

I don't like it personally and it breaks our existing style, so let's disable D209 for now.

bluetech

Great work @Pierre-Sassoulas, went through everything and it looks good to me.

One thing though, I tried it and it seems like ruff format doesn't support formatting code snippets in rst and md files yet. It does support formatting rst and md snippets in docstrings but that's not what we need. So until ruff implements this (astral-sh/ruff#8237), we should keep blacken-docs.

bluetech · 2024-02-01T20:52:55Z

testing/test_assertrewrite.py

@@ -1868,16 +1867,13 @@ def test_simple():
            id="assert statement not on own line",
        ),
        pytest.param(
-            b"def x():\n"


The changes here are definitely worse after, dunno if there's anything to be done though.

Oups, I missed this, I only searched for " f" and " ", after fixing it would be b"def x():\n assert (\n 1 + 2 # comment\n )\n", not extraordinary, but better I guess ?

Another way would be to change it to:

b"""def x(): assert ( 1 + 2 # comment ) """

And it seems to works for bytes.

Multiline b works but I think it will be annoying with the indentation.

Maybe just put # fmt: off/# fmt: on around them?

The scope of # fmt: off/# fmt: on might not be exactly the same in ruff vs black, I had to "un-nest" the existing fmt: off.

Pierre-Sassoulas · 2024-02-01T21:54:49Z

I rebased all the autofix commits to create the "one commit we can put in https://github.com/pytest-dev/pytest/blob/main/.git-blame-ignore-revs". Let me know what to do with all the manual fixes, squash all together or clean up the history.

bluetech · 2024-02-01T22:10:11Z

Let me know what to do with all the manual fixes, squash all together or clean up the history.

The rule I try to follow is "every commit should pass CI", this is very useful for bisects for example. The easiest way to achieve this here is to squash everything to one commit. If you want to preserve separate commits it will require a bit more effort.

Pierre-Sassoulas · 2024-02-01T22:18:32Z

All right, I can squash everything then upgrade the .git-blame-ignore-revs (it will be two commits that need to be merged, not squashde or rebased so the sha is accurate). Thank you for the reviews, there was a lot of changes (especially at first with bad configurations)

…e, to ruff ruff is faster and handle everything we had prior. isort configuration done based on the indication from astral-sh/ruff#4670, previousely based on reorder-python-import (pytest-dev#11896) flake8-docstrings was a wrapper around pydocstyle (now archived) that explicitly asks to use ruff in PyCQA/pydocstyle#658. flake8-typing-import is useful mainly for project that support python 3.7 and the one useful check will be implemented in astral-sh/ruff#2302 We need to keep blacken-doc because ruff does not handle detection of python code inside .md and .rst. The direct link to the repo is now used to avoid a redirection. Manual fixes: - Lines that became too long - % formatting that was not done automatically - type: ignore that were moved around - noqa of hard to fix issues (UP031 generally) - fmt: off and fmt: on that is not really identical between black and ruff - autofix re-order in pre-commit from faster to slower Co-authored-by: Ran Benita <ran@unusedvar.com>

…n-docs 2024's style

nicoddemus · 2024-02-02T13:13:00Z

@Pierre-Sassoulas thanks for the work on this! I think this should be backported to ease other backports.

Fix a duplicate assignment in test_config.py

046f647

Taken from pytest-dev#11885 that was closed.

bluetech reviewed Jan 31, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

pyproject.toml Outdated Show resolved Hide resolved

pyproject.toml Outdated Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

Pierre-Sassoulas changed the title ~~[git pre-commit hook] Migrate from isort to ruff~~ Migrate from autoflake, black, isort, some of pre-commit checks, pyupgrade, and flake8 to ruff Jan 31, 2024

Pierre-Sassoulas referenced this pull request in Pierre-Sassoulas/pytest Jan 31, 2024

Take https://github.com/pytest-dev/pytest/pull/11901\#discussion_r147…

9edcffe

…3034261 into account

Pierre-Sassoulas referenced this pull request in Pierre-Sassoulas/pytest Jan 31, 2024

Remove a default value

aa5f901

See https://github.com/pytest-dev/pytest/pull/11901\#discussion_r1473038296

Pierre-Sassoulas added a commit to Pierre-Sassoulas/pytest that referenced this pull request Jan 31, 2024

Remove useless option that change nothing

a40c468

See pytest-dev#11901 (comment)

Pierre-Sassoulas force-pushed the migrate-from-isort-to-ruff branch from 791543b to f300f7c Compare January 31, 2024 20:50

nicoddemus reviewed Jan 31, 2024

View reviewed changes

bluetech reviewed Jan 31, 2024

View reviewed changes

Pierre-Sassoulas changed the title ~~Migrate from autoflake, black, isort, some of pre-commit checks, pyupgrade, and flake8 to ruff~~ Migrate from autoflake, black, isort, pyupgrade, flake8 and pydocstyle, to ruff Feb 1, 2024

Pierre-Sassoulas force-pushed the migrate-from-isort-to-ruff branch from a550fa8 to 6ecc15d Compare February 1, 2024 12:05

Pierre-Sassoulas force-pushed the migrate-from-isort-to-ruff branch from 6ecc15d to 2c45d72 Compare February 1, 2024 13:34

Pierre-Sassoulas requested a review from nicoddemus February 1, 2024 17:03

Pierre-Sassoulas requested a review from bluetech February 1, 2024 17:03

bluetech reviewed Feb 1, 2024

View reviewed changes

Pierre-Sassoulas force-pushed the migrate-from-isort-to-ruff branch from 195f1ea to 3d53bbe Compare February 1, 2024 21:50

bluetech approved these changes Feb 1, 2024

View reviewed changes

Pierre-Sassoulas and others added 2 commits February 2, 2024 09:27

[.git-blame-ignore-revs] Add migration to ruff/ruff format and blacke…

9ef905e

…n-docs 2024's style

Pierre-Sassoulas force-pushed the migrate-from-isort-to-ruff branch from 7fa06f8 to 9ef905e Compare February 2, 2024 08:28

Pierre-Sassoulas enabled auto-merge February 2, 2024 08:30

Pierre-Sassoulas merged commit bdfc5c8 into pytest-dev:main Feb 2, 2024
24 checks passed

Pierre-Sassoulas deleted the migrate-from-isort-to-ruff branch February 2, 2024 08:57

Pierre-Sassoulas added the backport 8.0.x label Feb 2, 2024

This was referenced Feb 2, 2024

[backport] Migrate from autoflake, black, isort, pyupgrade, flake8 and pydocstyle, to ruff #11911

Merged

[pre-commit] Activate ruff checks and fix existing issues #11912

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate from `autoflake`, `black`, `isort`, `pyupgrade`, `flake8` and `pydocstyle`, to `ruff` #11901

Migrate from `autoflake`, `black`, `isort`, `pyupgrade`, `flake8` and `pydocstyle`, to `ruff` #11901

Pierre-Sassoulas commented Jan 31, 2024

nicoddemus commented Jan 31, 2024

Pierre-Sassoulas commented Jan 31, 2024

bluetech commented Jan 31, 2024

nicoddemus Jan 31, 2024

nicoddemus commented Jan 31, 2024

Pierre-Sassoulas commented Jan 31, 2024 •

edited

Loading

graingert commented Jan 31, 2024 •

edited

Loading

Pierre-Sassoulas commented Jan 31, 2024 •

edited

Loading

Pierre-Sassoulas commented Jan 31, 2024

bluetech Jan 31, 2024

Pierre-Sassoulas Jan 31, 2024

bluetech Feb 1, 2024

Pierre-Sassoulas Feb 1, 2024

Pierre-Sassoulas commented Jan 31, 2024 •

edited

Loading

bluetech commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024 •

edited

Loading

bluetech commented Feb 1, 2024

bluetech commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024

bluetech commented Feb 1, 2024 •

edited

Loading

bluetech left a comment

bluetech Feb 1, 2024

Pierre-Sassoulas Feb 1, 2024

Pierre-Sassoulas Feb 1, 2024 •

edited

Loading

bluetech Feb 1, 2024

Pierre-Sassoulas Feb 1, 2024

Pierre-Sassoulas Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024

bluetech commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024

nicoddemus commented Feb 2, 2024

Migrate from autoflake, black, isort, pyupgrade, flake8 and pydocstyle, to ruff #11901

Migrate from autoflake, black, isort, pyupgrade, flake8 and pydocstyle, to ruff #11901

Conversation

Pierre-Sassoulas commented Jan 31, 2024

nicoddemus commented Jan 31, 2024

Pierre-Sassoulas commented Jan 31, 2024

bluetech commented Jan 31, 2024

Choose a reason for hiding this comment

nicoddemus commented Jan 31, 2024

Pierre-Sassoulas commented Jan 31, 2024 • edited Loading

graingert commented Jan 31, 2024 • edited Loading

Pierre-Sassoulas commented Jan 31, 2024 • edited Loading

Pierre-Sassoulas commented Jan 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pierre-Sassoulas commented Jan 31, 2024 • edited Loading

bluetech commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024 • edited Loading

bluetech commented Feb 1, 2024

bluetech commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024

bluetech commented Feb 1, 2024 • edited Loading

bluetech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pierre-Sassoulas Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pierre-Sassoulas commented Feb 1, 2024

bluetech commented Feb 1, 2024

Pierre-Sassoulas commented Feb 1, 2024

nicoddemus commented Feb 2, 2024

Migrate from `autoflake`, `black`, `isort`, `pyupgrade`, `flake8` and `pydocstyle`, to `ruff` #11901

Migrate from `autoflake`, `black`, `isort`, `pyupgrade`, `flake8` and `pydocstyle`, to `ruff` #11901

Pierre-Sassoulas commented Jan 31, 2024 •

edited

Loading

graingert commented Jan 31, 2024 •

edited

Loading

Pierre-Sassoulas commented Jan 31, 2024 •

edited

Loading

Pierre-Sassoulas commented Jan 31, 2024 •

edited

Loading

Pierre-Sassoulas commented Feb 1, 2024 •

edited

Loading

bluetech commented Feb 1, 2024 •

edited

Loading

Pierre-Sassoulas Feb 1, 2024 •

edited

Loading