Merge pull request #1664 from DemocracyClub/feature/ruff

Ruffification

.circleci/config.yml

@@ -93,7 +93,7 @@ jobs:
           name: run pytest
           command: |
             . .venv/bin/activate
-            pytest -sx --flakes --cov-report= --cov=wcivf --junitxml=test-results/junit.xml
+            pytest -sx --ruff --cov-report= --cov=wcivf --junitxml=test-results/junit.xml
       - run: |
           if [ -n "$COVERALLS_REPO_TOKEN" ]; then
           . .venv/bin/activate

.circleci/tests/test_deploy.py

@@ -51,6 +51,7 @@ def get_output_value(stack, output_key):
         if output["OutputKey"] == output_key:
             # Cloudformation Outputs have unique per-stack names
             return output["OutputValue"]
+    return None
 
 
 @pytest.fixture

INSTALL.md

@@ -85,6 +85,14 @@ The project uses [Black](https://black.readthedocs.io/en/stable/) for code forma
 
     black .
 
+Additionally, this project uses [ruff](https://beta.ruff.rs/docs/) for linting. You can run it with:
+
+    ruff . 
+
+ruff has in-built functionality to fix common linting errors. Use the `--fix` option to do this.
+
+Both Black and ruff are both automatically called as part of pytest in this project.
+
 A pre-commit hook is defined in the project to run it automatically before each commit. See the [pre-commit docs](https://pre-commit.com/#quick-start) for more information, or simply run the below command to setup:
 
     pre-commit install

deploy/after_install/write_envfile.py

@@ -7,7 +7,6 @@
 
 import boto3
 
-
 client = boto3.client("ssm", region_name="eu-west-2")
 
 
@@ -33,9 +32,7 @@ def get_parameter_store_vars() -> Dict:
 
 def get_deploy_vars() -> Dict:
     with open(Path(__file__).parents[1] / "deploy-env-vars.json") as f:
-        deploy_vars = json.loads(f.read())
-
-    return deploy_vars
+        return json.loads(f.read())
 
 
 def write_parameters_to_envfile() -> None:

deploy/create_deployment.py

@@ -1,7 +1,8 @@
 import os
-import boto3
 import time
 
+import boto3
+
 session = boto3.Session()
 
 

deploy/create_deployment_group.py

@@ -1,7 +1,8 @@
 import os
+import time
+
 import boto3
 from botocore.exceptions import ClientError
-import time
 
 session = boto3.Session()
 
@@ -87,9 +88,9 @@ def create_default_asg():
         for asg in client.describe_auto_scaling_groups()["AutoScalingGroups"]
     ]
     if "default" in existing_asgs:
-        return
+        return None
 
-    response = client.create_auto_scaling_group(
+    return client.create_auto_scaling_group(
         AutoScalingGroupName="default",
         AvailabilityZones=[
             "eu-west-2a",
@@ -117,7 +118,6 @@ def create_default_asg():
         ],
         VPCZoneIdentifier=",".join(subnet_ids),
     )
-    return response
 
 
 def get_service_role():
@@ -191,6 +191,7 @@ def main():
     # step as the instance needs to have initialised and be in ready state
     # before code deploy can create a start deployment
     time.sleep(60)
+    return None
 
 
 if __name__ == "__main__":

deploy/update_auto_scaling_group.py

@@ -1,8 +1,8 @@
-from collections import defaultdict
 import os
-import boto3
 import time
+from collections import defaultdict
 
+import boto3
 
 client = boto3.client("autoscaling")
 
@@ -13,6 +13,7 @@ def _filter_by_code_deploy_tag(asg):
     """
     for tag in asg["Tags"]:
         return tag["Key"] == "CodeDeploy"
+    return None
 
 
 def get_latest_code_deploy_tagged_asg():

manage.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 import os
 import sys
+
 import dotenv
 
 if __name__ == "__main__":

pyproject.toml

@@ -1,2 +1,33 @@
 [tool.black]
 line-length = 80
+
+[tool.ruff]
+line-length = 80
+ignore = ["E501"]
+extend-select = [
+    "I",
+    "C4",
+    "SIM",
+    "Q003",
+    "RET",
+]
+extend-exclude = ["wcivf/settings/local.py"]
+
+[tool.pytest.ini_options]
+norecursedirs =[
+    "src",
+    "wcivf/static_root",
+    ".aws-sam",
+    ".ruff-cache",
+    ".circleci",
+]
+DJANGO_SETTINGS_MODULE = "wcivf.settings.base"
+addopts =[
+    "--ignore=*/__pycache__",
+    "--ignore=.*",
+    "--ignore=.download-cache",
+    "--ignore=wcivf/settings/local.py",
+    "--ignore=lib",
+    "--ruff",
+    "--black",
+]

requirements/testing.txt

@@ -4,8 +4,6 @@ black==23.7.0
 
 pytest
 pytest-django
-pytest-pep8
-pytest-flakes
 pytest-cov
 factory_boy
 vcrpy==5.0.0
@@ -16,3 +14,6 @@ tomlkit
 freezegun==1.2.2
 django-debug-toolbar==4.1.0
 curlylint==0.13.1
+ruff==0.0.280
+pytest-ruff
+pytest-black

wcivf/apps/api/permissions.py

@@ -7,3 +7,4 @@ def has_object_permission(self, request, view, obj):
         # so we'll always allow GET, HEAD or OPTIONS requests.
         if request.method in permissions.SAFE_METHODS:
             return True
+        return False

wcivf/apps/api/serializers.py

@@ -1,9 +1,8 @@
-from rest_framework import serializers
-
 from elections.models import VotingSystem
 from leaflets.api.serializers import LeafletSerializer
-from people.models import Person, PersonPost
 from parties.models import Party
+from people.models import Person, PersonPost
+from rest_framework import serializers
 
 
 class PersonSerializer(serializers.ModelSerializer):
@@ -33,6 +32,7 @@ def get_leaflets(self, obj: Person):
         leaflets = obj.ordered_leaflets[:4]
         if leaflets:
             return LeafletSerializer(leaflets, many=True, read_only=True).data
+        return None
 
 
 class PartySerializer(serializers.HyperlinkedModelSerializer):
@@ -78,6 +78,7 @@ def get_previous_party_affiliations(self, obj: PersonPost):
         parties = obj.previous_party_affiliations.all()
         if parties:
             return PartySerializer(parties, many=True, read_only=True).data
+        return None
 
 
 class VotingSystemSerializer(serializers.ModelSerializer):

wcivf/apps/api/tests/test_api.py

@@ -1,15 +1,14 @@
-from django.core.cache import cache
-from rest_framework.reverse import reverse
-from rest_framework.test import APITestCase
 import vcr
-
-from people.tests.factories import PersonFactory, PersonPostFactory
-from parties.tests.factories import PartyFactory
+from django.core.cache import cache
 from elections.tests.factories import (
     ElectionFactory,
-    PostFactory,
     PostElectionFactory,
+    PostFactory,
 )
+from parties.tests.factories import PartyFactory
+from people.tests.factories import PersonFactory, PersonPostFactory
+from rest_framework.reverse import reverse
+from rest_framework.test import APITestCase
 
 
 class TestAPIBasics(APITestCase):
@@ -133,4 +132,4 @@ def test_lock_status(self):
         url = reverse("api:candidates-for-postcode-list")
         req = self.client.get("{}?postcode=EC1A4EU".format(url))
         assert req.status_code == 200
-        assert req.json()[0]["ballot_locked"] == True
+        assert req.json()[0]["ballot_locked"] is True

wcivf/apps/api/urls.py

@@ -1,8 +1,6 @@
-from django.urls import path, include
-
-from rest_framework import routers
-
 from api import views
+from django.urls import include, path
+from rest_framework import routers
 
 router = routers.DefaultRouter()
 

wcivf/apps/api/views.py

@@ -1,20 +1,18 @@
 import abc
 
-from rest_framework import viewsets
-from rest_framework.exceptions import APIException
-from rest_framework.response import Response
-from rest_framework.views import APIView
-
-from django.conf import settings
-from django.utils.http import urlencode
-
 from api import serializers
 from api.serializers import VotingSystemSerializer
 from core.helpers import clean_postcode
+from django.conf import settings
+from django.utils.http import urlencode
+from elections.models import InvalidPostcodeError, PostElection
 from elections.views import mixins
-from elections.models import PostElection, InvalidPostcodeError
 from hustings.api.serializers import HustingSerializer
 from people.models import Person
+from rest_framework import viewsets
+from rest_framework.exceptions import APIException
+from rest_framework.response import Response
+from rest_framework.views import APIView
 
 
 class PostcodeNotProvided(APIException):
@@ -160,8 +158,7 @@ def get_ballots(self, request):
             pes = pes.modified_gt_with_related(date=modified_gt)
         else:
             pes = pes.order_by(*ordering)
-        ret = {"ballots": pes[:100]}
-        return ret
+        return {"ballots": pes[:100]}
 
 
 class LastUpdatedView(APIView):

wcivf/apps/core/context_processors.py

@@ -1,6 +1,6 @@
+from core.helpers import clean_postcode
 from django.conf import settings
 
-from core.helpers import clean_postcode
 from .forms import PostcodeLookupForm
 
 

wcivf/apps/core/forms.py

@@ -1,6 +1,5 @@
 from django import forms
 from django.utils.translation import gettext_lazy as _
-
 from localflavor.gb.forms import GBPostcodeField
 
 

wcivf/apps/core/helpers.py

@@ -1,8 +1,8 @@
+import json
 import re
 from contextlib import contextmanager
 from datetime import datetime, timedelta
 from urllib.parse import urlparse
-import json
 
 
 @contextmanager
@@ -46,8 +46,7 @@ def clean_postcode(postcode):
     postcode = postcode.replace("+", "")
     incode_pattern = "[0-9][ABD-HJLNP-UW-Z]{2}"
     space_regex = re.compile(r" *(%s)$" % incode_pattern)
-    postcode = space_regex.sub(r" \1", postcode.upper())
-    return postcode
+    return space_regex.sub(r" \1", postcode.upper())
 
 
 def twitter_username(url):

wcivf/apps/core/management/commands/init_data.py

@@ -2,13 +2,12 @@
 
 from django.conf import settings
 from django.core.cache import cache
-from django.core.management.base import BaseCommand
 from django.core.management import call_command
-
+from django.core.management.base import BaseCommand
 from elections.constants import (
     PEOPLE_FOR_BALLOT_KEY_FMT,
-    POSTCODE_TO_BALLOT_KEY_FMT,
     POLLING_STATIONS_KEY_FMT,
+    POSTCODE_TO_BALLOT_KEY_FMT,
 )
 
 
@@ -36,15 +35,14 @@ def handle(self, **options):
             print(" ".join(command))
             call_command(*command)
 
-        if options["full"]:
-            # Delete the cache on a full import
-            if hasattr(cache, "delete_pattern"):
-                for fmt in (
-                    POLLING_STATIONS_KEY_FMT,
-                    POSTCODE_TO_BALLOT_KEY_FMT,
-                    PEOPLE_FOR_BALLOT_KEY_FMT,
-                ):
-                    cache.delete_pattern(fmt.format("*"))
+        # Delete the cache on a full import
+        if options["full"] and hasattr(cache, "delete_pattern"):
+            for fmt in (
+                POLLING_STATIONS_KEY_FMT,
+                POSTCODE_TO_BALLOT_KEY_FMT,
+                PEOPLE_FOR_BALLOT_KEY_FMT,
+            ):
+                cache.delete_pattern(fmt.format("*"))
 
         # Unset dirty file if it exists
         if getattr(settings, "CHECK_HOST_DIRTY", False):

wcivf/apps/core/management/commands/process_log_queue.py

@@ -1,6 +1,5 @@
-from django.core.management.base import BaseCommand
-
 from core.models import write_logged_postcodes
+from django.core.management.base import BaseCommand
 
 
 class Command(BaseCommand):

wcivf/apps/core/management/commands/setup_django_site.py

@@ -2,8 +2,8 @@
 Hack because Django makes it hard to use data migrations to do this :/
 """
 
-from django.core.management.base import BaseCommand
 from django.contrib.sites.models import Site
+from django.core.management.base import BaseCommand
 
 
 class Command(BaseCommand):

wcivf/apps/core/management/commands/truncate_replicated_tables.py

@@ -1,8 +1,5 @@
 from django.core.management.base import BaseCommand
-
-
 from django.db import connection
-
 from elections.models import VotingSystem