From 8c529abe2940c10cc459ec2c49aa61ae4ab92398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Braghi=C8=99?= Date: Fri, 12 Apr 2024 12:21:38 +0100 Subject: [PATCH] Tooling updates (#388) * Upgrade ruff and use it for formatting as well * Lint * Move coverage config to .coveragerc * Add a Ruff GH action for inline annotations --- .coveragerc | 38 +++++++++++++++++++++++++++++++++ .github/workflows/ruff.yml | 21 ++++++++++++++++++ .pre-commit-config.yaml | 14 +++--------- README.md | 2 +- grapple/exceptions.py | 2 -- grapple/helpers.py | 4 +++- grapple/models.py | 14 ++++++------ grapple/settings.py | 1 + grapple/types/images.py | 1 + grapple/types/streamfield.py | 8 +++---- grapple/utils.py | 10 ++------- pyproject.toml | 41 ------------------------------------ ruff.toml | 25 ++++++++++++++-------- tests/settings.py | 18 ++-------------- tests/test_grapple.py | 2 +- tests/test_image_types.py | 8 +++---- tests/testapp/blocks.py | 7 +++--- tests/testapp/factories.py | 2 +- 18 files changed, 110 insertions(+), 108 deletions(-) create mode 100644 .coveragerc create mode 100644 .github/workflows/ruff.yml diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 00000000..6a37738c --- /dev/null +++ b/.coveragerc @@ -0,0 +1,38 @@ +[run] +branch = True +concurrency = multiprocessing, thread +parallel = True +source_pkgs = grapple +omit = **/migrations/*,docs/*,tests/* + +[paths] +source = grapple,.tox/py*/**/site-packages + +[report] +show_missing = True +ignore_errors = True +skip_covered = True +skip_empty = true + +# Regexes for lines to exclude from consideration +exclude_also = + # Have to re-enable the standard pragma + pragma: no cover + + # Don't complain about missing debug-only code: + def __repr__ + if self.debug + if settings.DEBUG + + # Don't complain if tests don't hit defensive assertion code: + raise AssertionError + raise NotImplementedError + + # Don't complain if non-runnable code isn't run: + if 0: + if __name__ == .__main__.: + + # Nor complain about type checking + "if TYPE_CHECKING:", + class .*\bProtocol\): + @(abc\.)?abstractmethod diff --git a/.github/workflows/ruff.yml b/.github/workflows/ruff.yml new file mode 100644 index 00000000..565da04c --- /dev/null +++ b/.github/workflows/ruff.yml @@ -0,0 +1,21 @@ +name: Ruff + +on: + push: + branches: + - main + - 'stable/**' + pull_request: + branches: [main] + +jobs: + ruff: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - run: python -Im pip install --user ruff + + - name: Run ruff + run: ruff --output-format=github grapple diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 62349996..c227f81d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,7 +4,7 @@ ci: repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.4.0 + rev: v4.6.0 hooks: - id: check-yaml - id: check-toml @@ -17,20 +17,12 @@ repos: - id: detect-private-key - id: end-of-file-fixer - id: check-added-large-files - - repo: https://github.com/psf/black-pre-commit-mirror - rev: 23.9.1 - hooks: - - id: black - repo: https://github.com/astral-sh/ruff-pre-commit - rev: 'v0.0.287' + rev: 'v0.3.7' hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] - - repo: https://github.com/adamchainz/blacken-docs - rev: 1.16.0 - hooks: - - id: blacken-docs - additional_dependencies: [black==23.9.1] + - id: ruff-format - repo: https://github.com/rtts/djhtml rev: 3.0.6 hooks: diff --git a/README.md b/README.md index 5c1adc33..b5f3ef25 100644 --- a/README.md +++ b/README.md @@ -7,8 +7,8 @@ # Wagtail Grapple [![Build status](https://github.com/torchbox/wagtail-grapple/actions/workflows/ci.yml/badge.svg)](https://github.com/torchbox/wagtail-grapple/actions) +[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) [![PyPi](https://img.shields.io/pypi/v/wagtail-grapple.svg)](https://pypi.org/project/wagtail-grapple/) -[![black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black) [![pre-commit.ci status](https://results.pre-commit.ci/badge/github/torchbox/wagtail-grapple/main.svg)](https://results.pre-commit.ci/latest/github/torchbox/wagtail-grapple/main) A library to build GraphQL endpoints easily so you can grapple your Wagtail data from anywhere! diff --git a/grapple/exceptions.py b/grapple/exceptions.py index 5ad55f10..7b3b2a11 100644 --- a/grapple/exceptions.py +++ b/grapple/exceptions.py @@ -4,5 +4,3 @@ class IllegalDeprecation(Exception): This is invalid - a deprecated entity must be nullable. """ - - pass diff --git a/grapple/helpers.py b/grapple/helpers.py index 5da2a4f8..c025fbc7 100644 --- a/grapple/helpers.py +++ b/grapple/helpers.py @@ -64,6 +64,7 @@ def register_field_middleware(field_name: str, middleware_list: list): def register_query_field( field_name, plural_field_name=None, + *, query_params=None, required=False, plural_required=False, @@ -184,6 +185,7 @@ def resolve_plural(self, _, info, **kwargs): def register_paginated_query_field( field_name, plural_field_name=None, + *, query_params=None, required=False, plural_required=False, @@ -300,7 +302,7 @@ def resolve_plural(self, _, info, **kwargs): def register_singular_query_field( - field_name, query_params=None, required=False, middleware=None + field_name, *, query_params=None, required=False, middleware=None ): def inner(cls): field_type = lambda: registry.models[cls] # noqa: E731 diff --git a/grapple/models.py b/grapple/models.py index d837acf8..5c8d9920 100644 --- a/grapple/models.py +++ b/grapple/models.py @@ -17,7 +17,12 @@ class GraphQLField: deprecation_reason: Optional[str] def __init__( - self, field_name: str, field_type: type = None, required: bool = None, **kwargs + self, + field_name: str, + field_type: type = None, + *, + required: bool = None, + **kwargs, ): # Initiate and get specific field info. self.field_name = field_name @@ -81,17 +86,14 @@ def Mixin(): (app_label, model) = snippet_model.lower().split(".") mdl = apps.get_model(app_label, model) - if mdl: - field_type = lambda: registry.snippets[mdl] # noqa: E731 - else: - field_type = graphene.String + field_type = (lambda: registry.snippets[mdl]) if mdl else graphene.String return GraphQLField(field_name, field_type, **kwargs) return Mixin -def GraphQLForeignKey(field_name, content_type, is_list=False, **kwargs): +def GraphQLForeignKey(field_name, content_type, **kwargs): def Mixin(): field_type = None if isinstance(content_type, str): diff --git a/grapple/settings.py b/grapple/settings.py index 2123d317..3b723042 100644 --- a/grapple/settings.py +++ b/grapple/settings.py @@ -9,6 +9,7 @@ Wagtail Grapple settings, checking for user settings first, then falling back to the defaults. """ + import logging from django.conf import settings as django_settings diff --git a/grapple/types/images.py b/grapple/types/images.py index baf50956..80e19141 100644 --- a/grapple/types/images.py +++ b/grapple/types/images.py @@ -197,6 +197,7 @@ def resolve_src_set( info: GraphQLResolveInfo, sizes: list[int], format: str | None = None, + *, preserve_svg: bool = True, **kwargs, ) -> str: diff --git a/grapple/types/streamfield.py b/grapple/types/streamfield.py index 67e3dca4..f62b9b74 100644 --- a/grapple/types/streamfield.py +++ b/grapple/types/streamfield.py @@ -172,11 +172,11 @@ def resolve_blocks(self, info, **kwargs): for field, value in stream_data.items(): block = dict(child_blocks)[field] - if issubclass(type(block), blocks.ChooserBlock) or not issubclass( - type(block), blocks.StructBlock + if isinstance(value, int) and ( + issubclass(type(block), blocks.ChooserBlock) + or not issubclass(type(block), blocks.StructBlock) ): - if isinstance(value, int): - value = block.to_python(value) + value = block.to_python(value) stream_blocks.append(StructBlockItem(field, block, value)) diff --git a/grapple/utils.py b/grapple/utils.py index 94cf74b0..ede62155 100644 --- a/grapple/utils.py +++ b/grapple/utils.py @@ -123,10 +123,7 @@ def resolve_queryset( :type collection: int """ - if id is not None: - qs = qs.filter(pk=id) - else: - qs = qs.all() + qs = qs.all() if id is None else qs.filter(pk=id) order_by_relevance = True if order is not None: @@ -216,10 +213,7 @@ def resolve_paginated_queryset( int(per_page or grapple_settings.PAGE_SIZE), grapple_settings.MAX_PAGE_SIZE ) - if id is not None: - qs = qs.filter(pk=id) - else: - qs = qs.all() + qs = qs.all() if id is None else qs.filter(pk=id) # order_by_relevance will always take precedence over an existing order_by in the Postgres backend # we need to set it to False if we want to specify our own order_by. diff --git a/pyproject.toml b/pyproject.toml index 973d7a39..72e61807 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -76,44 +76,3 @@ exclude = [ [tool.doc8] ignore = ["D000", "D001"] ignore_path = ["docs/build", "src", "tests", ".git", ".tox", ".venv", "venv"] - -[tool.coverage.run] -branch = true -parallel = true -concurrency = ["multiprocessing", "thread"] - -source = ["grapple"] - -omit = ["**/migrations/*", "docs/*", "tests/*", "testapp/*"] - -[tool.coverage.paths] -source = ["grapple", ".tox/py*/**/site-packages"] - -[tool.coverage.report] -omit = ["**/migrations/*", "docs/*", "tests/*", "testapp/*"] -show_missing = true -ignore_errors = true -skip_empty = true -skip_covered = true -exclude_lines = [ - # Have to re-enable the standard pragma - "pragma: no cover", - - # Don't complain about missing debug-only code: - "def __repr__", - "if self.debug", - - # Don't complain if tests don't hit defensive assertion code: - "raise AssertionError", - "raise NotImplementedError", - - # Don't complain if non-runnable code isn't run: - "if 0:", - "if __name__ == .__main__.:", - - # Don't complain about abstract methods, they aren't run: - "@(abc.)?abstractmethod", - - # Nor complain about type checking - "if TYPE_CHECKING:", -] diff --git a/ruff.toml b/ruff.toml index 936f4c54..77d45401 100644 --- a/ruff.toml +++ b/ruff.toml @@ -1,33 +1,40 @@ -target-version = "py38" +extend-exclude = ["build"] -exclude = ["dist","build","venv",".venv",".tox",".git"] - -select = [ +[lint] +extend-select = [ "B", # flake8-bugbear "BLE", # flake8-blind-except "C4", # flake8-comprehensions "DJ", # flake8-django "E", # pycodestyle errors "F", # pyflakes + "FBT", # flake8-boolean-trap "I", # isort + "PIE", # flake8-pie "PGH", # pygrep-hooks "RUF100", # unused noqa "S", # flake8-bandit + "SIM", # flake8-simplify "T20", # flake8-print "UP", # pyupgrade "W", # pycodestyle warnings "YTT", # flake8-2020 ] -fixable = ["C4", "E", "F", "I", "UP"] +extend-ignore = [ + "E501", # Line too long + "B028", # function-call-in-default-argument +] -# E501: Line too long -ignore = ["E501","B028"] +fixable = ["C4", "E", "F", "I", "UP"] -[per-file-ignores] +[lint.per-file-ignores] "tests/**.py" = ["DJ008"] -[isort] +[lint.isort] known-first-party = ["grapple"] lines-between-types = 1 lines-after-imports = 2 + +[format] +docstring-code-format = true diff --git a/tests/settings.py b/tests/settings.py index fca113b8..3fe7e8bc 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -1,16 +1,4 @@ -""" -Django settings for example project. - -Generated by 'django-admin startproject' using Django 2.0.8. - -For more information on this file, see -https://docs.djangoproject.com/en/4.2/topics/settings/ - -For the full list of settings and their values, see -https://docs.djangoproject.com/en/4.2/ref/settings/ -""" - -# Build paths inside the project like this: os.path.join(BASE_DIR, ...) +import contextlib import pathlib import dj_database_url @@ -199,7 +187,5 @@ EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend" -try: +with contextlib.suppress(ImportError): from .local import * # noqa: F403 -except ImportError: - pass diff --git a/tests/test_grapple.py b/tests/test_grapple.py index 69fef784..1c2445f9 100644 --- a/tests/test_grapple.py +++ b/tests/test_grapple.py @@ -570,7 +570,7 @@ def test_explicit_order(self): class PageUrlPathTest(BaseGrappleTest): - def _query_by_path(self, path, in_site=False): + def _query_by_path(self, path, *, in_site=False): query = """ query($urlPath: String, $inSite: Boolean) { page(urlPath: $urlPath, inSite: $inSite) { diff --git a/tests/test_image_types.py b/tests/test_image_types.py index feb0b235..4f78e6de 100644 --- a/tests/test_image_types.py +++ b/tests/test_image_types.py @@ -22,12 +22,12 @@ def setUpTestData(cls): def tearDown(self) -> None: for rendition in self.example_image.renditions.all(): - rendition.file.delete(False) + rendition.file.delete(save=False) @classmethod def tearDownClass(cls): super().tearDownClass() - cls.example_image.file.delete(False) + cls.example_image.file.delete(save=False) def test_query_url_field(self): query = """ @@ -284,12 +284,12 @@ def setUpTestData(cls): def tearDown(self) -> None: for rendition in self.example_svg_image.renditions.all(): - rendition.file.delete(False) + rendition.file.delete(save=False) @classmethod def tearDownClass(cls): super().tearDownClass() - cls.example_svg_image.file.delete(False) + cls.example_svg_image.file.delete(save=False) def test_schema_for_svg_related_fields_and_arguments(self): results = self.introspect_schema_by_type(Image.__name__) diff --git a/tests/testapp/blocks.py b/tests/testapp/blocks.py index d7788e10..0888e062 100644 --- a/tests/testapp/blocks.py +++ b/tests/testapp/blocks.py @@ -285,9 +285,10 @@ def get_link_url( """ Returns the page URL. """ - if page := values.get("page"): - if page_url := page.get_url(request=info.context): - return str(page_url) + if (page := values.get("page")) and ( + page_url := page.get_url(request=info.context) + ): + return str(page_url) return "" diff --git a/tests/testapp/factories.py b/tests/testapp/factories.py index 5f5daab1..35c46fde 100644 --- a/tests/testapp/factories.py +++ b/tests/testapp/factories.py @@ -213,7 +213,7 @@ def _create(cls, model_class, *args, **kwargs): message = ( f"Error building {model_class} with {cls.__name__}.\nBad values:\n" ) - for field in ve.error_dict.keys(): + for field in ve.error_dict: if field == "__all__": message += " __all__: obj.clean() failed\n" else: