Replace Python toolchain with Ruff (#3546)

Author: dhruvkb

.pre-commit-config.yaml

@@ -35,53 +35,13 @@ repos:
       - id: check-docstring-first
       - id: requirements-txt-fixer
 
-  # Use the `.isort.cfg` file to configure additional project-specific requirements.
-  - repo: https://github.com/PyCQA/isort
-    rev: 5.12.0
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.1.8
     hooks:
-      - id: isort
-        files: \.py$
-        exclude: ^build/.*$|^.tox/.*$|^venv/.*$
-        args:
-          - --profile=black
-          - --lines-after-imports=2
-
-  - repo: https://github.com/asottile/pyupgrade
-    rev: v3.15.0
-    hooks:
-      - id: pyupgrade
-        args:
-          - --py310-plus
-
-  - repo: https://github.com/charliermarsh/ruff-pre-commit
-    rev: "v0.1.7"
-    hooks:
-      - id: ruff
+      - id: ruff # replaces Flake8, isort, pydocstyle, pyupgrade
         args:
           - --fix
-
-  - repo: https://github.com/ambv/black
-    rev: 23.11.0
-    hooks:
-      - id: black
-        args:
-          - --safe
-
-  - repo: https://github.com/pycqa/pydocstyle
-    rev: 6.3.0
-    hooks:
-      - id: pydocstyle
-        args:
-          - --convention=pep257
-          # Additional ignore reasons:
-          # D1xx: we do not want to force contributors to write redundant or useless docstrings
-          # D202: additional whitespace helps with readability
-          # D205: we don't want to always require a single line summary
-          # D211: same as D202
-          # D400: first line doesn't need to end in a period
-          # See the following documentation for what each rule does:
-          # https://www.pydocstyle.org/en/6.2.3/error_codes.html#error-codes
-          - --add-ignore=D1,D202,D205,D211,D400
+      - id: ruff-format # replaces Black
 
   # Use the `.prettierignore` and `.prettier.config.js` files to configure project-specific requirements.
   - repo: https://github.com/pre-commit/mirrors-prettier

api/api/views/image_views.py

@@ -153,7 +153,7 @@ async def thumbnail(self, *args, **kwargs):
     @action(detail=True, url_path="watermark", url_name="watermark")
     def watermark(self, request, *_, **__):  # noqa: D401
         """
-        This endpoint is deprecated.
+        Note that this endpoint is deprecated.
 
         ---
 

api/test/unit/controllers/test_search_controller.py

@@ -618,9 +618,7 @@ def test_no_post_process_results_recursion(
     # Ensure dead link filtering does not remove any results
     pook.head(
         pook.regex(rf"{MOCK_LIVE_RESULT_URL_PREFIX}/\d"),
-    ).times(
-        hit_count
-    ).reply(200)
+    ).times(hit_count).reply(200)
 
     serializer = image_media_type_config.search_request_serializer(
         # This query string does not matter, ultimately, as pook is mocking

api/test/unit/utils/test_image_proxy.py

@@ -59,9 +59,7 @@ def auth_key():
 
 @pytest.fixture
 def photon_get(session_loop):
-    """
-    Run ``image_proxy.get`` and wait for all tasks to finish.
-    """
+    """Run ``image_proxy.get`` and wait for all tasks to finish."""
 
     def do(*args, **kwargs):
         try:

api/test/unit/utils/test_watermark.py

@@ -17,9 +17,9 @@
 @dataclass
 class RequestsFixture:
     requests: list[Request]
-    response_factory: Callable[  # noqa: E731
-        [Request], Response
-    ] = lambda x: RequestsFixture._default_response_factory(x)
+    response_factory: Callable[[Request], Response] = (  # noqa: E731
+        lambda x: RequestsFixture._default_response_factory(x)
+    )
 
     @staticmethod
     def _default_response_factory(req: Request) -> Response:

api/test/unit/utils/test_waveform.py

@@ -19,9 +19,9 @@
 @dataclass
 class RequestsFixture:
     requests: list[Request]
-    response_factory: Callable[  # noqa: E731
-        [Request], Response
-    ] = lambda x: RequestsFixture._default_response_factory(x)
+    response_factory: Callable[[Request], Response] = (  # noqa: E731
+        lambda x: RequestsFixture._default_response_factory(x)
+    )
 
     @staticmethod
     def _default_response_factory(req: Request) -> Response:

api/test/unit/views/test_media_views.py

@@ -32,9 +32,7 @@ def test_list_query_count(api_client, media_type_config):
     ), patch(
         "api.serializers.media_serializers.search_controller",
         get_sources=MagicMock(return_value={}),
-    ), pytest_django.asserts.assertNumQueries(
-        1
-    ):
+    ), pytest_django.asserts.assertNumQueries(1):
         res = api_client.get(f"/v1/{media_type_config.url_prefix}/")
 
     assert res.status_code == 200

catalog/dags/common/loader/reporting.py

@@ -93,8 +93,7 @@ def skip_report_completion(
 ) -> bool:
     return (
         # Duration must be provided and be a value greater than 1 second
-        duration is None
-        or duration in ("inf", "less than 1 sec")
+        duration is None or duration in ("inf", "less than 1 sec")
     ) and (
         # Record counts by media type must be provided and at least one value must
         # be truthy (i.e. not None)

catalog/tests/dags/common/loader/test_sql.py

@@ -85,8 +85,8 @@ def postgres_with_load_and_image_table(load_table, sql_info, mock_pg_hook_task):
 
 def _load_local_tsv(tmpdir, bucket, tsv_file_name, identifier, mock_pg_hook_task):
     """
-    This wraps sql.load_local_data_to_intermediate_table so we can test it
-    under various conditions.
+    Wrap ``sql.load_local_data_to_intermediate_table`` so we can test it under
+    various conditions.
     """
     tsv_file_path = os.path.join(RESOURCES, tsv_file_name)
     with open(tsv_file_path) as f:

catalog/tests/dags/common/test_log_cleanup.py

@@ -46,8 +46,10 @@ def calculate_cutoffs():
 
 
 def test_log_cleaner_leaves_new_files():
-    """If all the log files are newer than the maxLogAgeInDays,
-    no log files are deleted"""
+    """
+    If all the log files are newer than the maxLogAgeInDays,
+    no log files are deleted
+    """
     log_files_count = len(list(Path.glob(logs_folder, "**/*.log")))
     assert log_files_count == INITIAL_LOG_FILE_COUNT
 
@@ -61,8 +63,10 @@ def test_log_cleaner_leaves_new_files():
 
 
 def test_log_cleaner_deletes_only_old_files():
-    """Log cleaner deletes all the log files that are older than
-    maxLogAgeInDays, but leaves the files that are newer"""
+    """
+    Log cleaner deletes all the log files that are older than
+    maxLogAgeInDays, but leaves the files that are newer
+    """
     deleted_folders = log_cleanup.clean_up(
         logs_folder, cutoffs_in_days[1], ENABLE_DELETE
     )

catalog/tests/dags/providers/provider_api_scripts/test_cc_mixter.py

@@ -1,6 +1,4 @@
-"""
-Run these tests locally with `just test -k cc_mixter`
-"""
+"""Run these tests locally with `just test -k cc_mixter`"""
 
 import json
 from pathlib import Path

catalog/tests/dags/providers/provider_api_scripts/test_provider_data_ingester.py

@@ -395,7 +395,7 @@ def test_ingest_records_uses_query_params_list_from_dagrun_conf():
 
 
 def test_ingest_records_raises_IngestionError():
-    with (patch.object(ingester, "get_batch") as get_batch_mock,):
+    with patch.object(ingester, "get_batch") as get_batch_mock:
         get_batch_mock.side_effect = [
             Exception("Mock exception message"),
             (EXPECTED_BATCH_DATA, True),  # Second batch should not be reached

catalog/tests/dags/providers/provider_api_scripts/test_time_delineated_provider_data_ingester.py

@@ -91,14 +91,15 @@ def test_get_timestamp_pairs_returns_full_day_when_few_records_found():
 def test_get_timestamp_pairs_with_large_record_counts():
     with patch.object(ingester, "_get_record_count") as mock_count:
         # Mock the calls to _get_record_count in order
-        mock_count.side_effect = [
-            150_000,  # Getting total count for the entire day
-            0,  # Get count for first hour, count == 0
-            10,  # Get count for second hour, count < max_records
-            101_000,  # Get count for third hour, count > division_threshold
-            49_090,  # Get count for fourth hour, max_records < count < division_threshold
-        ] + list(
-            repeat(0, 20)
+        mock_count.side_effect = (
+            [
+                150_000,  # Getting total count for the entire day
+                0,  # Get count for first hour, count == 0
+                10,  # Get count for second hour, count < max_records
+                101_000,  # Get count for third hour, count > division_threshold
+                49_090,  # Get count for fourth hour, max_records < count < division_threshold
+            ]
+            + list(repeat(0, 20))
         )  # Fill list with count == 0 for the remaining hours
 
         # We only get timestamp pairs for the hours that had records. For the

catalog/tests/dags/providers/test_factory_utils.py

@@ -23,6 +23,8 @@ def dagrun_mock() -> DagRun:
 @pytest.fixture
 def internal_func_mock():
     """
+    Get an empty ``MagicMock`` instance.
+
     This mock, along with the value, get handed into the provided function.
     For fake_provider_module.main, the mock will be called with the provided value.
     """

catalog/tests/utilities/dag_doc_gen/test_dag_doc_generation.py

@@ -185,7 +185,7 @@ def test_generate_dag_doc():
 this one has a doc
 """
     )
-    with (mock.patch(f"{_MODULE}.get_dags_info") as get_dags_info_mock,):
+    with mock.patch(f"{_MODULE}.get_dags_info") as get_dags_info_mock:
         # Return in reverse order to ensure they show up in the correct order
         get_dags_info_mock.return_value = [
             DagInfo("b", None, "this one has a doc", "t1", False, None),

documentation/_ext/link_issues.py

@@ -122,7 +122,7 @@ def apply(self) -> None:
                 if len(match.groups()) != 1:
                     raise ValueError(
                         "issuetracker_issue_pattern must have "
-                        "exactly one group: {!r}".format(match.groups())
+                        f"exactly one group: {match.groups()!r}"
                     )
 
                 # extract the text between the last issue reference and the

pyproject.toml

@@ -6,29 +6,39 @@
 exclude-file = ".codespell/ignore_lines.txt"
 ignore-words = ".codespell/ignore_words.txt"
 
-[tool.ruff.per-file-ignores]
-"*test*" = ["E501"]
-"*__init__*" = ["F401"]
-"*settings/__init__.py" = ["F403"]
+[tool.ruff.lint]
+# Default for `select` is ["E4", "E7", "E9", "F"]
+# All additional rules must be added using `extend-select`.
+extend-select = [
+  "UP",
+  "D",  # pydocstyle, https://docs.astral.sh/ruff/rules/#pydocstyle-d
+]
+ignore = [
+  "D1",  # D1xx: we do not want to force contributors to write redundant or useless docstrings
+  "D202",  # D202: additional whitespace helps with readability
+  "D205",  # D205: we don't want to always require a single line summary
+  "D211",  # D211: same as D202
+  "D400",  # D400: first line doesn't need to end in a period
+]
 
-[tool.isort]
-profile = "black"
+[tool.ruff.lint.per-file-ignores]
+"*test*" = ["E501"]  # Tests should have long, descriptive names.
+"*__init__*" = ["F401"]  # Init files usually have unused imports.
 
-sections = [
-  "FUTURE",
-  "STDLIB",
-  "DJANGO",
-  "THIRDPARTY",
-  "FIRSTPARTY",
-  "LOCALFOLDER",
-]
+[tool.ruff.lint.pydocstyle]
+convention = "pep257"
 
-known_django = [
+[tool.ruff.lint.isort]
+lines-after-imports = 2
+section-order = [
+  "future",
+  "standard-library",
   "django",
-  "rest_framework",
+  "third-party",
+  "first-party",
+  "local-folder",
 ]
-
-known_first_party = [
+known-first-party = [
   "conf",
   "api",
   "ingestion_server",
@@ -41,4 +51,5 @@ known_first_party = [
   "retired",
 ]
 
-lines_after_imports = 2
+[tool.ruff.lint.isort.sections]
+django = ["django", "rest_framework"]