Merge pull request #26 from lsst-sqre/tickets/DM-40096

DM-40096: Use Ruff for linting

.pre-commit-config.yaml

@@ -6,31 +6,20 @@ repos:
       - id: check-yaml
       - id: check-toml
 
-  - repo: https://github.com/pycqa/isort
-    rev: 5.12.0
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.0.278
     hooks:
-      - id: isort
-        additional_dependencies: [toml]
+      - id: ruff
+        args: [--fix, --exit-non-zero-on-fix]
 
   - repo: https://github.com/psf/black
-    rev: 23.1.0
+    rev: 23.7.0
     hooks:
       - id: black
 
   - repo: https://github.com/asottile/blacken-docs
-    rev: 1.13.0
+    rev: 1.15.0
     hooks:
       - id: blacken-docs
-        additional_dependencies: [black==23.1.0]
-        args: [-l, '79', -t, py38]
-
-  - repo: https://github.com/pycqa/pydocstyle
-    rev: 6.3.0
-    hooks:
-    - id: pydocstyle
-      additional_dependencies: [tomli]
-
-  - repo: https://github.com/pycqa/flake8
-    rev: 6.0.0
-    hooks:
-      - id: flake8
+        additional_dependencies: [black==23.7.0]
+        args: [-l, '79', -t, py310]

changelog.d/20230718_175341_jsick_ruff.md

@@ -0,0 +1,4 @@
+### Other changes
+
+- Use ruff for linting the codebase, replacing flake8 and isort.
+- Improve the codebase following ruff's recommendations.

docs/_rst_epilog.rst

@@ -10,3 +10,4 @@
 .. _Schema Evolution and Compatibility: https://docs.confluent.io/current/schema-registry/avro.html
 .. _Strimzi: https://strimzi.io
 .. _tox: https://tox.readthedocs.io/en/latest/
+.. _scriv: https://scriv.readthedocs.io/en/latest/

docs/dev/development.rst

@@ -43,8 +43,8 @@ Pre-commit hooks
 The pre-commit hooks, which are automatically installed by running the :command:`make init` command on :ref:`set up <dev-environment>`, ensure that files are valid and properly formatted.
 Some pre-commit hooks automatically reformat code:
 
-``isort``
-    Automatically sorts imports in Python modules.
+``ruff``
+    Automatically fixes common issues in code and sorts imports.
 
 ``black``
     Automatically formats Python code.
@@ -99,29 +99,17 @@ Updating the change log
 =======================
 
 Each pull request should update the change log (:file:`CHANGELOG.md`).
-Add a description of new features and fixes as list items under a section at the top of the change log called "Unreleased:"
+The change log is maintained with scriv_.
 
-.. code-block:: md
+To create a new change log fragment, run:
 
-   ## Unreleased
-
-   - Description of the feature or fix.
-
-If the next version is known (because Kafkit's main branch is being prepared for a new major or minor version), the section may contain that version information:
-
-.. code-block:: md
-
-   ## X.Y.0 (unreleased)
-
-   - Description of the feature or fix.
-
-If the exact version and release date is known (:doc:`because a release is being prepared <release>`), the section header is formatted as:
-
-.. code-block:: rst
+.. code-block:: sh
 
-   ## X.Y.0 (YYYY-MM-DD)
+   scriv create
 
-   - Description of the feature or fix.
+This creates a new file in the :file:`changelog.d` directory.
+Edit this file to describe the changes in the pull request.
+If sections don't apply to the change you can delete them.
 
 .. _style-guide:
 
@@ -131,7 +119,7 @@ Style guide
 Code
 ----
 
-- The code style follows :pep:`8`, though in practice lean on Black and isort to format the code for you.
+- The code style follows :pep:`8`, though in practice lean on Black and ruff to format the code for you.
 
 - Use :pep:`484` type annotations.
   The ``tox -e typing`` test environment, which runs mypy_, ensures that the project's types are consistent.

docs/dev/release.rst

@@ -26,11 +26,16 @@ Release tags are semantic version identifiers following the :pep:`440` specifica
 1. Change log and documentation
 -------------------------------
 
-Each PR should include updates to the change log.
+Each PR should include updates to the change log as scriv_ fragments (see :ref:`dev-change-log`).
+When a release is being made, collect these fragments into the change log by running:
+
+.. code-block:: sh
+
+   scriv collect --version "X.Y.Z"
+
 If the change log or documentation needs additional updates, now is the time to make those changes through the regular branch-and-PR development method against the ``main`` branch.
 
-In particular, replace the "Unreleased" section headline with the semantic version and date.
-See :ref:`dev-change-log` in the *Developer guide* for details.
+Each PR should have already created scriv_ change log fragments.
 
 2. Tag the release
 ------------------

pyproject.toml

@@ -97,31 +97,6 @@ exclude = '''
 # Use single-quoted strings so TOML treats the string like a Python r-string
 # Multi-line strings are implicitly treated by black as regular expressions
 
-[tool.pydocstyle]
-# Reference: http://www.pydocstyle.org/en/stable/error_codes.html
-convention = "numpy"
-add_select = [
-    "D212", # Multi-line docstring summary should start at the first line
-]
-add-ignore = [
-    "D105", # Missing docstring in magic method
-    "D102", # Missing docstring in public method (needed for docstring inheritance)
-    "D100", # Missing docstring in public module
-    # Below are required to allow multi-line summaries.
-    "D200", # One-line docstring should fit on one line with quotes
-    "D205", # 1 blank line required between summary line and description
-    "D400", # First line should end with a period
-    # Properties shouldn't be written in imperative mode. This will be fixed
-    # post 6.1.1, see https://github.com/PyCQA/pydocstyle/pull/546
-    "D401",
-]
-
-[tool.isort]
-profile = "black"
-line_length = 79
-known_first_party = "kafkit"
-skip = ["docs/conf.py"]
-
 [tool.pytest]
 
 [tool.pytest.ini_options]
@@ -140,6 +115,93 @@ warn_redundant_casts = true
 warn_unreachable = true
 warn_unused_ignores = true
 
+# The rule used with Ruff configuration is to disable every lint that has
+# legitimate exceptions that are not dodgy code, rather than cluttering code
+# with noqa markers. This is therefore a reiatively relaxed configuration that
+# errs on the side of disabling legitimate lints.
+#
+# Reference for settings: https://beta.ruff.rs/docs/settings/
+# Reference for rules: https://beta.ruff.rs/docs/rules/
+[tool.ruff]
+exclude = [
+    "docs/**",
+]
+line-length = 79
+ignore = [
+    "ANN101",  # self should not have a type annotation
+    "ANN102",  # cls should not have a type annotation
+    "ANN401",  # sometimes Any is the right type
+    "ARG001",  # unused function arguments are often legitimate
+    "ARG002",  # unused method arguments are often legitimate
+    "ARG005",  # unused lambda arguments are often legitimate
+    "BLE001",  # we want to catch and report Exception in background tasks
+    "C414",    # nested sorted is how you sort by multiple keys with reverse
+    "COM812",  # omitting trailing commas allows black autoreformatting
+    "D102",    # sometimes we use docstring inheritence
+    "D104",    # don't see the point of documenting every package
+    "D105",    # our style doesn't require docstrings for magic methods
+    "D106",    # Pydantic uses a nested Config class that doesn't warrant docs
+    "D205",    # Allow multi-line summary sentences
+    "EM101",   # justification (duplicate string in traceback) is silly
+    "EM102",   # justification (duplicate string in traceback) is silly
+    "FBT003",  # positional booleans are normal for Pydantic field defaults
+    "G004",    # forbidding logging f-strings is appealing, but not our style
+    "RET505",  # disagree that omitting else always makes code more readable
+    "PLR0913", # factory pattern uses constructors with many arguments
+    "PLR2004", # too aggressive about magic values
+    "S105",    # good idea but too many false positives on non-passwords
+    "S106",    # good idea but too many false positives on non-passwords
+    "SIM102",  # sometimes the formatting of nested if statements is clearer
+    "SIM116",  # allow if-else-if-else chains
+    "SIM117",  # sometimes nested with contexts are clearer
+    "TCH001",  # we decided to not maintain separate TYPE_CHECKING blocks
+    "TCH002",  # we decided to not maintain separate TYPE_CHECKING blocks
+    "TCH003",  # we decided to not maintain separate TYPE_CHECKING blocks
+    "TID252",  # if we're going to use relative imports, use them always
+    "TRY003",  # good general advice but lint is way too aggressive
+]
+select = ["ALL"]
+target-version = "py310"
+
+[tool.ruff.per-file-ignores]
+"tests/**" = [
+    "D103",    # tests don't need docstrings
+    "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
+    "SLF001",  # tests are allowed to access private members
+    "T201",    # Print is ok in tests
+]
+
+[tool.ruff.isort]
+known-first-party = ["kafkit", "tests"]
+split-on-trailing-comma = false
+
+# These are too useful as attributes or methods to allow the conflict with the
+# built-in to rule out their use.
+[tool.ruff.flake8-builtins]
+builtins-ignorelist = [
+    "all",
+    "any",
+    "help",
+    "id",
+    "list",
+    "type",
+]
+
+[tool.ruff.flake8-pytest-style]
+fixture-parentheses = false
+mark-parentheses = false
+
+[tool.ruff.pep8-naming]
+classmethod-decorators = [
+    "pydantic.root_validator",
+    "pydantic.validator",
+]
+
+[tool.ruff.pydocstyle]
+convention = "numpy"
+
 [tool.scriv]
 categories = [
     "Backwards-incompatible changes",

src/kafkit/__init__.py

@@ -1,10 +1,10 @@
-"""Kafkit helps you write Kafka producers and consumers in Python with asyncio.
+"""Kafkit helps you write Kafka producers and consumers in Python
+with asyncio.
 """
 
 __all__ = ["__version__", "version_info"]
 
 from importlib.metadata import PackageNotFoundError, version
-from typing import List
 
 __version__: str
 """The version string of Kafkit (PEP 440 / SemVer compatible)."""
@@ -15,7 +15,7 @@
     # package is not installed
     __version__ = "0.0.0"
 
-version_info: List[str] = __version__.split(".")
+version_info: list[str] = __version__.split(".")
 """The decomposed version, split across "``.``."
 
 Use this for version comparison.

src/kafkit/fastapi/dependencies/pydanticschemamanager.py

@@ -3,7 +3,6 @@
 """
 
 from collections.abc import Iterable
-from typing import Type
 
 from dataclasses_avroschema.avrodantic import AvroBaseModel
 from httpx import AsyncClient
@@ -30,7 +29,7 @@ async def initialize(
         *,
         http_client: AsyncClient,
         registry_url: str,
-        models: Iterable[Type[AvroBaseModel]],
+        models: Iterable[type[AvroBaseModel]],
         suffix: str = "",
         compatibility: str = "FORWARD",
     ) -> None:

src/kafkit/httputils.py

@@ -6,7 +6,7 @@
 
 import cgi
 import urllib.parse
-from typing import Mapping, Optional, Tuple
+from collections.abc import Mapping
 
 import uritemplate
 
@@ -39,8 +39,8 @@ def format_url(*, host: str, url: str, url_vars: Mapping[str, str]) -> str:
 
 
 def parse_content_type(
-    content_type: Optional[str],
-) -> Tuple[Optional[str], str]:
+    content_type: str | None,
+) -> tuple[str | None, str]:
     """Tease out the content-type and character encoding.
 
     A default character encoding of UTF-8 is used, so the content-type

src/kafkit/registry/aiohttp.py

@@ -6,7 +6,8 @@
 
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Mapping, Tuple
+from collections.abc import Mapping
+from typing import TYPE_CHECKING
 
 from kafkit.registry import sansio
 
@@ -33,7 +34,7 @@ def __init__(self, *, session: ClientSession, url: str) -> None:
 
     async def _request(
         self, method: str, url: str, headers: Mapping[str, str], body: bytes
-    ) -> Tuple[int, Mapping[str, str], bytes]:
+    ) -> tuple[int, Mapping[str, str], bytes]:
         async with self._session.request(
             method, url, headers=headers, data=body
         ) as response:

src/kafkit/registry/errors.py

@@ -9,7 +9,7 @@
     "UnmanagedSchemaError",
 ]
 
-from typing import Any, Optional
+from typing import Any
 
 
 class RegistryError(Exception):
@@ -37,8 +37,8 @@ def __init__(
         self,
         status_code: int,
         *args: Any,
-        error_code: Optional[int] = None,
-        message: Optional[str] = None,
+        error_code: int | None = None,
+        message: str | None = None,
     ) -> None:
         self.status_code = status_code
         self.error_code = error_code

src/kafkit/registry/httpx.py

@@ -6,7 +6,7 @@
 
 from __future__ import annotations
 
-from typing import Mapping, Tuple
+from collections.abc import Mapping
 
 from httpx import AsyncClient
 
@@ -30,7 +30,7 @@ def __init__(self, *, http_client: AsyncClient, url: str) -> None:
 
     async def _request(
         self, method: str, url: str, headers: Mapping[str, str], body: bytes
-    ) -> Tuple[int, Mapping[str, str], bytes]:
+    ) -> tuple[int, Mapping[str, str], bytes]:
         response = await self._client.request(
             method, url, headers=headers, content=body
         )