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

Adopting ruff formatter #7930

Merged
merged 17 commits into from Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
Expand Up @@ -25,13 +25,13 @@ rebuild-lockfiles: .pdm

.PHONY: format ## Auto-format python source files
format: .pdm
pdm run black $(sources)
pdm run ruff --fix $(sources)
sydney-runkle marked this conversation as resolved.
Show resolved Hide resolved
pdm run ruff format $(sources)

.PHONY: lint ## Lint python source files
lint: .pdm
pdm run ruff $(sources)
pdm run black $(sources) --check --diff
pdm run ruff format --check $(sources)

.PHONY: codespell ## Use Codespell to do spellchecking
codespell: .pre-commit
Expand Down
4 changes: 2 additions & 2 deletions docs/contributing.md
Expand Up @@ -90,8 +90,8 @@ Run tests and linting locally to make sure everything is working as expected.
```bash
# Run automated code formatting and linting
make format
# Pydantic uses black and ruff
# (https://github.com/ambv/black, https://github.com/charliermarsh/ruff)
# Pydantic uses ruff, an awesome Python linter written in rust
# https://github.com/astral-sh/ruff

# Run tests and linting
make
Expand Down
42 changes: 0 additions & 42 deletions pdm.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pydantic/_internal/_model_construction.py
Expand Up @@ -367,8 +367,8 @@ def inspect_namespace( # noqa C901
)
else:
raise PydanticUserError(
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',
Comment on lines -370 to 373
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, your diligence is impressive and I appreciate it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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']

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

)
Expand Down
8 changes: 4 additions & 4 deletions pydantic/color.py
Expand Up @@ -55,13 +55,13 @@ def __getitem__(self, item: Any) -> Any:
r_hex_short = r'\s*(?:#|0x)?([0-9a-f])([0-9a-f])([0-9a-f])([0-9a-f])?\s*'
r_hex_long = r'\s*(?:#|0x)?([0-9a-f]{2})([0-9a-f]{2})([0-9a-f]{2})([0-9a-f]{2})?\s*'
# CSS3 RGB examples: rgb(0, 0, 0), rgba(0, 0, 0, 0.5), rgba(0, 0, 0, 50%)
r_rgb = fr'\s*rgba?\(\s*{_r_255}{_r_comma}{_r_255}{_r_comma}{_r_255}(?:{_r_comma}{_r_alpha})?\s*\)\s*'
r_rgb = rf'\s*rgba?\(\s*{_r_255}{_r_comma}{_r_255}{_r_comma}{_r_255}(?:{_r_comma}{_r_alpha})?\s*\)\s*'
# CSS3 HSL examples: hsl(270, 60%, 50%), hsla(270, 60%, 50%, 0.5), hsla(270, 60%, 50%, 50%)
r_hsl = fr'\s*hsla?\(\s*{_r_h}{_r_comma}{_r_sl}{_r_comma}{_r_sl}(?:{_r_comma}{_r_alpha})?\s*\)\s*'
r_hsl = rf'\s*hsla?\(\s*{_r_h}{_r_comma}{_r_sl}{_r_comma}{_r_sl}(?:{_r_comma}{_r_alpha})?\s*\)\s*'
# CSS4 RGB examples: rgb(0 0 0), rgb(0 0 0 / 0.5), rgb(0 0 0 / 50%), rgba(0 0 0 / 50%)
r_rgb_v4_style = fr'\s*rgba?\(\s*{_r_255}\s+{_r_255}\s+{_r_255}(?:\s*/\s*{_r_alpha})?\s*\)\s*'
r_rgb_v4_style = rf'\s*rgba?\(\s*{_r_255}\s+{_r_255}\s+{_r_255}(?:\s*/\s*{_r_alpha})?\s*\)\s*'
# CSS4 HSL examples: hsl(270 60% 50%), hsl(270 60% 50% / 0.5), hsl(270 60% 50% / 50%), hsla(270 60% 50% / 50%)
r_hsl_v4_style = fr'\s*hsla?\(\s*{_r_h}\s+{_r_sl}\s+{_r_sl}(?:\s*/\s*{_r_alpha})?\s*\)\s*'
r_hsl_v4_style = rf'\s*hsla?\(\s*{_r_h}\s+{_r_sl}\s+{_r_sl}(?:\s*/\s*{_r_alpha})?\s*\)\s*'

# colors where the two hex characters are the same, if all colors match this the short version of hex colors can be used
repeat_colors = {int(c * 2, 16) for c in '0123456789abcdef'}
Expand Down
19 changes: 14 additions & 5 deletions pydantic/deprecated/class_validators.py
Expand Up @@ -114,13 +114,13 @@ def validator(
fields = tuple((__field, *fields))
if isinstance(fields[0], FunctionType):
raise PydanticUserError(
"`@validator` should be used with fields and keyword arguments, not bare. "
'`@validator` should be used with fields and keyword arguments, not bare. '
"E.g. usage should be `@validator('<field_name>', ...)`",
code='validator-no-fields',
)
elif not all(isinstance(field, str) for field in fields):
raise PydanticUserError(
"`@validator` fields should be passed as separate string args. "
'`@validator` fields should be passed as separate string args. '
"E.g. usage should be `@validator('<field_name_1>', '<field_name_2>', ...)`",
code='validator-invalid-fields',
)
Expand Down Expand Up @@ -162,7 +162,10 @@ def root_validator(
# which means you need to specify `skip_on_failure=True`
skip_on_failure: Literal[True],
allow_reuse: bool = ...,
) -> Callable[[_V1RootValidatorFunctionType], _V1RootValidatorFunctionType,]:
) -> Callable[
[_V1RootValidatorFunctionType],
_V1RootValidatorFunctionType,
]:
...


Expand All @@ -173,7 +176,10 @@ def root_validator(
# `skip_on_failure`, in fact it is not allowed as an argument!
pre: Literal[True],
allow_reuse: bool = ...,
) -> Callable[[_V1RootValidatorFunctionType], _V1RootValidatorFunctionType,]:
) -> Callable[
[_V1RootValidatorFunctionType],
_V1RootValidatorFunctionType,
]:
...


Expand All @@ -185,7 +191,10 @@ def root_validator(
pre: Literal[False],
skip_on_failure: Literal[True],
allow_reuse: bool = ...,
) -> Callable[[_V1RootValidatorFunctionType], _V1RootValidatorFunctionType,]:
) -> Callable[
[_V1RootValidatorFunctionType],
_V1RootValidatorFunctionType,
]:
...


Expand Down
2 changes: 1 addition & 1 deletion pydantic/main.py
Expand Up @@ -1110,7 +1110,7 @@ def parse_file( # noqa: D102

@classmethod
@typing_extensions.deprecated(
"The `from_orm` method is deprecated; set "
'The `from_orm` method is deprecated; set '
"`model_config['from_attributes']=True` and use `model_validate` instead.",
category=PydanticDeprecatedSince20,
)
Expand Down
2 changes: 1 addition & 1 deletion pydantic/networks.py
Expand Up @@ -636,7 +636,7 @@ def _validate(cls, __input_value: NetworkType) -> IPv4Network | IPv6Network:

def _build_pretty_email_regex() -> re.Pattern[str]:
name_chars = r'[\w!#$%&\'*+\-/=?^_`{|}~]'
unquoted_name_group = fr'((?:{name_chars}+\s+)*{name_chars}+)'
unquoted_name_group = rf'((?:{name_chars}+\s+)*{name_chars}+)'
quoted_name_group = r'"((?:[^"]|\")+)"'
email_group = r'<\s*(.+)\s*>'
return re.compile(rf'\s*(?:{unquoted_name_group}|{quoted_name_group})?\s*{email_group}\s*')
Expand Down
23 changes: 5 additions & 18 deletions pyproject.toml
Expand Up @@ -84,14 +84,12 @@ docs = [
"pyupgrade",
"mike @ git+https://github.com/jimporter/mike.git",
"mkdocs-embed-external-markdown>=2.3.0",
"black>=23.3.0",
"pytest-examples>=0.0.10",
"pydantic-settings>=2.0b1",
"pydantic-extra-types @ git+https://github.com/pydantic/pydantic-extra-types.git@main"
]
linting = [
"black",
"ruff",
"ruff=0.1.3",
"mypy~=1.1.1",
]
testing = [
Expand Down Expand Up @@ -176,8 +174,7 @@ unconfirmed_label = 'pending'
[tool.ruff]
sydney-runkle marked this conversation as resolved.
Show resolved Hide resolved
line-length = 120
extend-select = ['Q', 'RUF100', 'C90', 'UP', 'I', 'D', 'T']
# E501 is handled by black
extend-ignore = ['D105', 'D107', 'D205', 'D415', 'E501']
extend-ignore = ['D105', 'D107', 'D205', 'D415']
flake8-quotes = {inline-quotes = 'single', multiline-quotes = 'double'}
mccabe = { max-complexity = 14 }
isort = { known-first-party = ['pydantic', 'tests'] }
Expand All @@ -190,6 +187,9 @@ extend-exclude = ['pydantic/v1', 'tests/mypy/outputs']
"tests/benchmarks/test_fastapi_startup_generics.py" = ['UP006', 'UP007']
"tests/benchmarks/test_fastapi_startup_simple.py" = ['UP006', 'UP007']

[tool.ruff.format]
quote-style = 'single'

[tool.ruff.pydocstyle]
convention = "google"

Expand Down Expand Up @@ -228,19 +228,6 @@ source = [
'D:\a\pydantic\pydantic\pydantic',
]

[tool.black]
color = true
line-length = 120
target-version = ['py310']
skip-string-normalization = true
exclude = '''
(
/(
pydantic/v1
| tests/mypy/outputs
)/
)
'''

[tool.pyright]
include = ['pydantic']
Expand Down
4 changes: 2 additions & 2 deletions tests/check_usage_docs.py
Expand Up @@ -10,7 +10,7 @@
version_file = PYDANTIC_DIR / 'version.py'


version = re.search(br"VERSION = '(.*)'", version_file.read_bytes()).group(1)
version = re.search(rb"VERSION = '(.*)'", version_file.read_bytes()).group(1)
version_major_minor = b'.'.join(version.split(b'.')[:2])
expected_base = b'https://docs.pydantic.dev/' + version_major_minor + b'/'

Expand All @@ -30,7 +30,7 @@ def sub(m: re.Match) -> bytes:
else:
return m.group(0)

b = re.sub(br'(""" *usage.docs: *)(https://.+?/.+?/)', sub, b, flags=re.I)
b = re.sub(rb'(""" *usage.docs: *)(https://.+?/.+?/)', sub, b, flags=re.I)
if changed:
error_count += changed
path.write_bytes(b)
Expand Down
4 changes: 2 additions & 2 deletions tests/test_discriminated_union.py
Expand Up @@ -361,7 +361,7 @@ class Top(BaseModel):
'ctx': {'discriminator': "'m'", 'expected_tags': '1, 2', 'tag': '3'},
'input': {'m': 3},
'loc': ('sub',),
'msg': "Input tag '3' found using 'm' does not match any of the expected " "tags: 1, 2",
'msg': "Input tag '3' found using 'm' does not match any of the expected " 'tags: 1, 2',
'type': 'union_tag_invalid',
}
]
Expand Down Expand Up @@ -911,7 +911,7 @@ def test_wrap_function_schema() -> None:
def test_plain_function_schema_is_invalid() -> None:
with pytest.raises(
TypeError,
match="'function-plain' is not a valid discriminated union variant; " "should be a `BaseModel` or `dataclass`",
match="'function-plain' is not a valid discriminated union variant; " 'should be a `BaseModel` or `dataclass`',
):
apply_discriminator(
core_schema.union_schema(
Expand Down
8 changes: 4 additions & 4 deletions tests/test_edge_cases.py
Expand Up @@ -1127,7 +1127,7 @@ class C(A):
TypeError,
match=(
"Field 'integer' defined on a base class was overridden by a non-annotated attribute. "
"All field definitions, including overrides, require a type annotation."
'All field definitions, including overrides, require a type annotation.'
),
):

Expand Down Expand Up @@ -1197,8 +1197,8 @@ def test_unable_to_infer():
with pytest.raises(
errors.PydanticUserError,
match=re.escape(
"A non-annotated attribute was detected: `x = None`. All model fields require a type annotation; "
"if `x` is not meant to be a field, you may be able to resolve this error by annotating it as a "
'A non-annotated attribute was detected: `x = None`. All model fields require a type annotation; '
'if `x` is not meant to be a field, you may be able to resolve this error by annotating it as a '
"`ClassVar` or updating `model_config['ignored_types']`"
),
):
Expand Down Expand Up @@ -1541,7 +1541,7 @@ class Model(BaseModel):

model_config = dict(arbitrary_types_allowed=True)

assert re.search(fr'\(annotation={re.escape(expected)},', str(Model.model_fields))
assert re.search(rf'\(annotation={re.escape(expected)},', str(Model.model_fields))


def test_any_none():
Expand Down
2 changes: 1 addition & 1 deletion tests/test_generics.py
Expand Up @@ -268,7 +268,7 @@ class Result(BaseModel):

assert str(exc_info.value) == (
"<class 'tests.test_generics.test_must_inherit_from_generic.<locals>.Result'> cannot be "
"parametrized because it does not inherit from typing.Generic"
'parametrized because it does not inherit from typing.Generic'
)


Expand Down
4 changes: 2 additions & 2 deletions tests/test_main.py
Expand Up @@ -1511,8 +1511,8 @@ def test_untyped_fields_warning():
with pytest.raises(
PydanticUserError,
match=re.escape(
"A non-annotated attribute was detected: `x = 1`. All model fields require a type annotation; "
"if `x` is not meant to be a field, you may be able to resolve this error by annotating it "
'A non-annotated attribute was detected: `x = 1`. All model fields require a type annotation; '
'if `x` is not meant to be a field, you may be able to resolve this error by annotating it '
"as a `ClassVar` or updating `model_config['ignored_types']`."
),
):
Expand Down
1 change: 0 additions & 1 deletion tests/test_networks.py
Expand Up @@ -805,7 +805,6 @@ def test_address_valid(value, name, email):
('\u0020@example.com', None),
('\u001f@example.com', None),
('"@example.com', None),
('\"@example.com', None),
dmontagu marked this conversation as resolved.
Show resolved Hide resolved
(',@example.com', None),
('foobar <foobar<@example.com>', None),
('foobar <foobar@example.com>>', None),
Expand Down
2 changes: 1 addition & 1 deletion tests/test_plugins.py
Expand Up @@ -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}}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 😄

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'json success result=a=2',
]
log.clear()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_private_attributes.py
Expand Up @@ -289,7 +289,7 @@ class Model(ParentAModel, ParentBModel):
def test_private_attributes_not_dunder() -> None:
with pytest.raises(
NameError,
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__'.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

):

class MyModel(BaseModel):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_types.py
Expand Up @@ -4852,7 +4852,7 @@ def test_union_uuid_str_left_to_right():

# in smart mode JSON and python are currently validated differently in this
# case, because in Python this is a str but in JSON a str is also a UUID
assert TypeAdapter(IdOrSlug).validate_json('\"f4fe10b4-e0c8-4232-ba26-4acd491c2414\"') == UUID(
assert TypeAdapter(IdOrSlug).validate_json('"f4fe10b4-e0c8-4232-ba26-4acd491c2414"') == UUID(
'f4fe10b4-e0c8-4232-ba26-4acd491c2414'
)
assert (
Expand All @@ -4863,7 +4863,7 @@ def test_union_uuid_str_left_to_right():
IdOrSlugLTR = Annotated[Union[UUID, str], Field(union_mode='left_to_right')]

# in left to right mode both JSON and python are validated as UUID
assert TypeAdapter(IdOrSlugLTR).validate_json('\"f4fe10b4-e0c8-4232-ba26-4acd491c2414\"') == UUID(
assert TypeAdapter(IdOrSlugLTR).validate_json('"f4fe10b4-e0c8-4232-ba26-4acd491c2414"') == UUID(
'f4fe10b4-e0c8-4232-ba26-4acd491c2414'
)
assert TypeAdapter(IdOrSlugLTR).validate_python('f4fe10b4-e0c8-4232-ba26-4acd491c2414') == UUID(
Expand Down