Adopting `ruff` formatter #7930

Luca-Blight · 2023-10-25T21:09:18Z

Change Summary

Ruff is now the sole linter 👍
Files Altered: ['Makefile', 'docs/contributing.md', 'pyproject.toml', 'pdm.lock' ]

Notes:

For pyproject.toml file, I removed the last element('E501') of line 180 given black's removal but I'm not totally sure what this line is for.

Related Issue Number

Fix #7914

Checklist

The pull request title is a good summary of the changes - it will be used in the changelog
Unit tests for the changes exist
Tests pass on CI
Documentation reflects the changes where applicable
My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Luca-Blight · 2023-10-25T22:03:59Z

I suppose I could run Ruff local to fix the linting issues...what do you think @sydney-runkle?

sydney-runkle · 2023-10-25T23:37:51Z

Hi @Luca-Blight,

Looks good! Yeah, go ahead and run the linter locally to fix the failing checks. I'll take another look after you do that.

Thanks 🌟

sydney-runkle

@Luca-Blight,

Thanks for running the linter locally. I suggested a couple of very minor changes 👍

pyproject.toml

Makefile

pydantic/_internal/_decorators.py

sydney-runkle · 2023-10-26T00:32:19Z

pydantic/_internal/_model_construction.py

-                    f"A non-annotated attribute was detected: `{var_name} = {value!r}`. All model fields require a "
-                    f"type annotation; if `{var_name}` is not meant to be a field, you may be able to resolve this "
+                    f'A non-annotated attribute was detected: `{var_name} = {value!r}`. All model fields require a '
+                    f'type annotation; if `{var_name}` is not meant to be a field, you may be able to resolve this '
                    f"error by annotating it as a `ClassVar` or updating `model_config['ignored_types']`.",
                    code='model-field-missing-annotation',


This change also seems odd to me, as it's not changing the last line. I wonder what happens when we use:

[tool.ruff.flake8-quotes] multiline-quotes = 'double'

In the pyproject.toml

Actually, it should be 'double' by default, so I don't think that'll change anything. This modification still strikes me as odd though.

Yea good eye there. Let me try the change out and see what happens.

Btw, your diligence is impressive and I appreciate it.

It looks like

[tool.ruff.flake8-quotes] multiline-quotes = 'double'

is already implemented on line 179:

[tool.ruff] line-length = 120 extend-select = ['Q', 'RUF100', 'C90', 'UP', 'I', 'D', 'T'] extend-ignore = ['D105', 'D107', 'D205', 'D415'] flake8-quotes = {inline-quotes = 'single', multiline-quotes = 'double'} mccabe = { max-complexity = 14 } isort = { known-first-party = ['pydantic', 'tests'] } target-version = "py37" extend-exclude = ['pydantic/v1', 'tests/mypy/outputs']

Good catch. I'll leave this as unresolved for now given that I'm still curious about the change the ruff formatter made, but I don't think it's a problem in terms of the flake8-quotes configuration.

This strikes me as a bug -- we should be using double quotes for multi-line strings regardless of the formatter quote setting. Will look into it shortly and report back.

The last line doesn't get changed because changing the quote style would require escaping the '. See my other comment below.

Sorry, as usual, Micha is right -- this is an implicit concatenation, not a multiline string. So it's trying to use single quotes for each segment, and then skipping those that contain a single quote.

Ah, I see. This happened a few times with implicit concatenations, and I thought the pattern was odd at first glance bc it always seemed to be skipping the last line, but that seems to always be where we have a single quote based on the nature of many of our strings. Thanks!

tests/test_networks.py

sydney-runkle · 2023-10-26T00:44:27Z

tests/test_private_attributes.py

-        match="Private attributes must not use dunder names;" " use a single underscore prefix instead of '__foo__'.",
+        match='Private attributes must not use dunder names;' " use a single underscore prefix instead of '__foo__'.",


Clearly it's having a hard time with this as well 😆

Yea that is weird. I wonder what it's tripping on...

So, what's happening here is that it's trying to convert each segment to single quotes (as per quote-style = 'single'). It's succeeding for the first segment in this implicit concatenation, but it avoids changing the second segment, because that segment itself contains a single quote (and so that single quote would need to be escaped).

Arguably we should be making that decision across the entire implicit concatenation (i.e., looking across all segments) to avoid these mixed quotes. I'll file an issue and we'll discuss.

Ruff changes the quotes of the first string but not of the second because it would require escaping the '. Is it unexpected that Ruff uses different quotes for different parts of the implicit concatenated string? Would you prefer if it would change the quotes consistently for the entire implicit concatenated string?

Ah, thanks for the explanation. Clearly I missed the pattern of escaping ' that was at the root of many of my questions.

I do feel like changing quotes consistently for an entire implicit concatenated string might look a bit cleaner, but I'm not adamant about the change. Excited to hear what others think.

@charliermarsh, thanks for filing an issue 👍

@charliermarsh could you link the issue here? I may have just missed it, but didn't see it at a first glance

Apologies -- just filed it here: astral-sh/ruff#8265

sydney-runkle · 2023-10-26T01:04:18Z

@Luca-Blight,

Thanks for your work on this 💯. Hopefully we can resolve the few odd formatting patterns that we're seeing tomorrow and get this across the line 😄.

sydney-runkle · 2023-10-26T02:09:40Z

@charliermarsh @MichaReiser,

Thanks for your super prompt support. Looks like we'll have this across the line in no time 🚀 !

sydney-runkle

LGTM 👍

The only thing I'm still seeing is the implicit concatenation changes that we discussed already, which I don't think we have to address in this PR, but can continue discussion with ruff and change this down the line.

@Luca-Blight, great work!

Luca-Blight · 2023-10-26T20:48:38Z

LGTM 👍

The only thing I'm still seeing is the implicit concatenation changes that we discussed already, which I don't think we have to address in this PR, but can continue discussion with ruff and change this down the line.

@Luca-Blight, great work!

I noticed some letter swapping in some of the files such as rf -> fr and br -> rb.
There are several examples in the test files and color.py. Do you see that as an issue?

sydney-runkle · 2023-10-26T20:50:25Z

I noticed some letter swapping in some of the files such as rf -> fr and br -> rb. There are several examples in the test files and color.py. Do you see that as an issue?

Yeah I saw that as well. I don't think it's an issue. @adriangb or @dmontagu, do you think otherwise?

adriangb · 2023-10-26T21:11:34Z

I see it's changing rf'a string' to fr'a string'. I agree it doesn't matter.

tests/test_networks.py

dmontagu · 2023-10-26T21:22:05Z

tests/test_plugins.py

@@ -284,7 +284,7 @@ class Model(BaseModel):
        assert Model.model_validate_json('{"a": 2}', context={'c': 2}).model_dump() == {'a': 2}
        # insert_assert(log)
        assert log == [
-            'json enter input={"a": 2} kwargs={\'strict\': None, \'context\': {\'c\': 2}}',
+            "json enter input={\"a\": 2} kwargs={'strict': None, 'context': {'c': 2}}",


@charliermarsh if you are looking for feedback, I'd think that it would be preferable to use single quotes on the outer level when single quotes are preferred and the string contains both single and double quotes. But I definitely don't care enough to hold off on merging just for this issue 😄

Yeah I agree — this one struck me as odd. Gonna look into it.

astral-sh/ruff#8264

Luca-Blight and others added 4 commits October 25, 2023 16:49

Removed black

142876d

Merge branch 'pydantic:main' into main

3061734

line 28 fix

0d5a88a

Merge branch 'main' of https://github.com/Luca-Blight/pydantic

44959bc

Luca-Blight changed the title ~~Removal of Black WIP~~ Removal of Black Oct 25, 2023

sydney-runkle added the relnotes-feature label Oct 25, 2023

ruff reformatting

aa02e81

sydney-runkle requested changes Oct 26, 2023

View reviewed changes

pyproject.toml Show resolved Hide resolved

pyproject.toml Outdated Show resolved Hide resolved

pydantic-hooky bot added awaiting author revision labels Oct 26, 2023

pydantic-hooky bot assigned Luca-Blight Oct 26, 2023

ruff reformatting modification

fe237ff

sydney-runkle requested changes Oct 26, 2023

View reviewed changes

Makefile Show resolved Hide resolved

sydney-runkle reviewed Oct 26, 2023

View reviewed changes

pydantic/_internal/_decorators.py Outdated Show resolved Hide resolved

sydney-runkle reviewed Oct 26, 2023

View reviewed changes

tests/test_networks.py Outdated Show resolved Hide resolved

sydney-runkle reviewed Oct 26, 2023

View reviewed changes

sydney-runkle changed the title ~~Removal of Black~~ Adopting ruff formatter Oct 26, 2023

ruff --fix changed to format

f0c8dd4

sydney-runkle added 3 commits October 26, 2023 08:36

removing redundant test case

e8598a4

minor changes, adding other ruff commands to format and lint steps

d5e5795

switching lint and format order

4b8c2d7

Luca-Blight mentioned this pull request Oct 26, 2023

Replace current linting packages with Ruff guardrails-ai/guardrails#403

Open

Luca-Blight added 3 commits October 26, 2023 16:14

revert fe237ff

03d21de

revert aa02e81

1a9924e

pin new ruff version + single quote-style

8cb3a46

Luca-Blight and others added 2 commits October 26, 2023 16:34

ruff formatting v0.1.3

b1c15ca

removing redundant test

21338f1

sydney-runkle approved these changes Oct 26, 2023

View reviewed changes

Luca-Blight added 2 commits October 26, 2023 16:41

typo fix

c36a29a

Merge branch 'main' of https://github.com/Luca-Blight/pydantic

7def92f

dmontagu reviewed Oct 26, 2023

View reviewed changes

tests/test_networks.py Show resolved Hide resolved

dmontagu reviewed Oct 26, 2023

View reviewed changes

dmontagu approved these changes Oct 26, 2023

View reviewed changes

sydney-runkle merged commit 419398d into pydantic:main Oct 26, 2023
59 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adopting `ruff` formatter #7930

Adopting `ruff` formatter #7930

Luca-Blight commented Oct 25, 2023 •

edited by sydney-runkle

Luca-Blight commented Oct 25, 2023

sydney-runkle commented Oct 25, 2023

sydney-runkle left a comment •

edited

sydney-runkle Oct 26, 2023

sydney-runkle Oct 26, 2023

Luca-Blight Oct 26, 2023

Luca-Blight Oct 26, 2023

Luca-Blight Oct 26, 2023

sydney-runkle Oct 26, 2023

charliermarsh Oct 26, 2023

MichaReiser Oct 26, 2023

charliermarsh Oct 26, 2023

sydney-runkle Oct 26, 2023

sydney-runkle Oct 26, 2023

Luca-Blight Oct 26, 2023

charliermarsh Oct 26, 2023

MichaReiser Oct 26, 2023

sydney-runkle Oct 26, 2023

sydney-runkle Oct 26, 2023

charliermarsh Oct 26, 2023

sydney-runkle commented Oct 26, 2023

sydney-runkle commented Oct 26, 2023

sydney-runkle left a comment

Luca-Blight commented Oct 26, 2023

sydney-runkle commented Oct 26, 2023

adriangb commented Oct 26, 2023

dmontagu Oct 26, 2023

charliermarsh Oct 26, 2023

charliermarsh Oct 26, 2023

		match="Private attributes must not use dunder names;" " use a single underscore prefix instead of '__foo__'.",
		match='Private attributes must not use dunder names;' " use a single underscore prefix instead of '__foo__'.",

Adopting ruff formatter #7930

Adopting ruff formatter #7930

Conversation

Luca-Blight commented Oct 25, 2023 • edited by sydney-runkle

Change Summary

Related Issue Number

Checklist

Luca-Blight commented Oct 25, 2023

sydney-runkle commented Oct 25, 2023

sydney-runkle left a comment • edited

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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sydney-runkle commented Oct 26, 2023

sydney-runkle commented Oct 26, 2023

sydney-runkle left a comment

Choose a reason for hiding this comment

Luca-Blight commented Oct 26, 2023

sydney-runkle commented Oct 26, 2023

adriangb commented Oct 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Adopting `ruff` formatter #7930

Adopting `ruff` formatter #7930

Luca-Blight commented Oct 25, 2023 •

edited by sydney-runkle

sydney-runkle left a comment •

edited