Skip to content

Commit

Permalink
Update Ruff configuration
Browse files Browse the repository at this point in the history
Use Ruff for formatting as well as linting, and drop Black for code.
Update the version of Black used for documentation and apply the
formatting changes it wants to make.
  • Loading branch information
rra committed Mar 11, 2024
1 parent b41caf2 commit 18a5e26
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 23 deletions.
8 changes: 2 additions & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,11 @@ repos:
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]

- repo: https://github.com/ambv/black
rev: 24.2.0
hooks:
- id: black
- id: ruff-format

- repo: https://github.com/asottile/blacken-docs
rev: 1.16.0
hooks:
- id: blacken-docs
additional_dependencies: [black==23.1.0]
additional_dependencies: [black==24.2.0]
args: [-l, '76', -t, py311]
6 changes: 3 additions & 3 deletions docs/user-guide/arq.rst
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ If your app uses a configuration system like ``pydantic.BaseSettings``, this exa
redis_settings = RedisSettings(
host=url_parts.hostname or "localhost",
port=url_parts.port or 6379,
database=int(url_parts.path.lstrip("/"))
if url_parts.path
else 0,
database=(
int(url_parts.path.lstrip("/")) if url_parts.path else 0
),
)
return redis_settings
Expand Down
3 changes: 1 addition & 2 deletions docs/user-guide/fastapi-errors.rst
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,7 @@ For example, for a single route:
...,
responses={404: {"description": "Not found", "model": ErrorModel}},
)
async def route(foo: str) -> None:
...
async def route(foo: str) -> None: ...
If all routes provided by a router have the same error handling behavior for a given response code, it saves some effort to instead do this when including the router, normally in ``main.py``:

Expand Down
27 changes: 24 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,13 @@ ignore = [
"FIX002", # point of a TODO comment is that we're not ready to fix it
"G004", # forbidding logging f-strings is appealing, but not our style
"RET505", # disagree that omitting else always makes code more readable
"PLR0911", # often many returns is clearer and simpler style
"PLR0913", # factory pattern uses constructors with many arguments
"PLR2004", # too aggressive about magic values
"PLW0603", # yes global is discouraged but if needed, it's needed
"S105", # good idea but too many false positives on non-passwords
"S106", # good idea but too many false positives on non-passwords
"S107", # good idea but too many false positives on non-passwords
"S603", # not going to manually mark every subprocess call as reviewed
"S607", # using PATH is not a security vulnerability
"SIM102", # sometimes the formatting of nested if statements is clearer
Expand All @@ -225,14 +228,31 @@ ignore = [
"TD003", # we don't require issues be created for TODOs
"TID252", # if we're going to use relative imports, use them always
"TRY003", # good general advice but lint is way too aggressive
"TRY301", # sometimes raising exceptions inside try is the best flow

# Safir-specific rules.
"N818", # Exception is correct in some cases, others are part of API
"PLW0603", # necessary trick for safir.logging
# The following settings should be disabled when using ruff format
# per https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules
"W191",
"E111",
"E114",
"E117",
"D206",
"D300",
"Q000",
"Q001",
"Q002",
"Q003",
"COM812",
"COM819",
"ISC001",
"ISC002",
]
select = ["ALL"]

[tool.ruff.lint.per-file-ignores]
"src/safir/**" = [
"N818", # Exception is correct in some cases, others are part of API
]
"src/safir/testing/**" = [
"S101", # test support functions are allowed to use assert
]
Expand All @@ -243,6 +263,7 @@ select = ["ALL"]
"PLR0915", # tests are allowed to be long, sometimes that's convenient
"PT012", # way too aggressive about limiting pytest.raises blocks
"S101", # tests should use assert
"S106", # tests are allowed to hard-code dummy passwords
"SLF001", # tests are allowed to access private members
]

Expand Down
2 changes: 1 addition & 1 deletion src/safir/asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def qsize(self) -> int:


def run_with_asyncio(
f: Callable[P, Coroutine[None, None, T]]
f: Callable[P, Coroutine[None, None, T]],
) -> Callable[P, T]:
"""Run the decorated function with `asyncio.run`.
Expand Down
4 changes: 2 additions & 2 deletions src/safir/dependencies/gafaelfawr.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@


async def auth_dependency(
x_auth_request_user: Annotated[str, Header(include_in_schema=False)]
x_auth_request_user: Annotated[str, Header(include_in_schema=False)],
) -> str:
"""Retrieve authentication information from HTTP headers.
Expand All @@ -27,7 +27,7 @@ async def auth_dependency(


async def auth_delegated_token_dependency(
x_auth_request_token: Annotated[str, Header(include_in_schema=False)]
x_auth_request_token: Annotated[str, Header(include_in_schema=False)],
) -> str:
"""Retrieve Gafaelfawr delegated token from HTTP headers.
Expand Down
2 changes: 1 addition & 1 deletion src/safir/pydantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class Model(BaseModel):


def _copy_type(
parent: Callable[P, T]
parent: Callable[P, T],
) -> Callable[[Callable[..., T]], Callable[P, T]]:
"""Copy the type of a parent method.
Expand Down
3 changes: 2 additions & 1 deletion src/safir/testing/kubernetes.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ class _KubernetesModel(Protocol):
"""

def to_dict(
self, serialize: bool = False # noqa: FBT001, FBT002
self,
serialize: bool = False, # noqa: FBT001, FBT002
) -> dict[str, Any]: ...


Expand Down
2 changes: 1 addition & 1 deletion tests/dependencies/gafaelfawr_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ async def test_auth_dependency() -> None:

@app.get("/")
async def handler(
user: Annotated[str, Depends(auth_dependency)]
user: Annotated[str, Depends(auth_dependency)],
) -> dict[str, str]:
return {"user": user}

Expand Down
4 changes: 2 additions & 2 deletions tests/logging_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def test_dev_exception_logging(caplog: LogCaptureFixture) -> None:
logger = structlog.get_logger("myapp")

try:
raise ValueError("this is some exception") # noqa: TRY301
raise ValueError("this is some exception")
except Exception:
logger.exception("exception happened", foo="bar")

Expand All @@ -226,7 +226,7 @@ def test_production_exception_logging(caplog: LogCaptureFixture) -> None:
logger = structlog.get_logger("myapp")

try:
raise ValueError("this is some exception") # noqa: TRY301
raise ValueError("this is some exception")
except Exception:
logger.exception("exception happened", foo="bar")

Expand Down
2 changes: 1 addition & 1 deletion tests/middleware/ivoa_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ async def simple_handler() -> dict[str, str]:

@app.get("/list")
async def list_handler(
param: Annotated[list[str], Query()]
param: Annotated[list[str], Query()],
) -> dict[str, list[str]]:
return {"param": param}

Expand Down

0 comments on commit 18a5e26

Please sign in to comment.