Introduce ruff to ci (#2051)

* Initial attempt at ruff

* prohibit FIXME, TODO and XXX

* add zip strict parameter

* add ignores in settings

* add __all__

* add stacklevel to warning, when top level code is executed twice

* add DJ checks

* ruff formatter

* enable "free" tests

* enable and (auto) fix set|list|map literal

* enable and (auto) fix implicit string concat

* enable and (auto) fix unnecessary start in range

* enable and (auto) fix exception parentheses

* enable and (auto) fix unnecessary assignment

* enable COMpatible COM

* enable and fix unspecific ignore

* enable and fix string join

* enable and fix list comprehensions

* attr defined mypy ignore

* enable PL, but keep invalid-name

* enable PERF203 and BLE instead of broad-exception-caught

* Use correct weekday range

* run ruff first and then pylint. Also do this with manage.py

* do not print in commands, use stdout instead!

* lint enrollment_preprocessor


---------

Co-authored-by: Richard Ebeling <[email protected]>

.github/workflows/tests.yml

@@ -88,7 +88,10 @@ jobs:
       - name: Setup python
         uses: ./.github/setup_python
 
-      - name: Run linter
+      - name: Run ruff
+        run: ruff check .
+
+      - name: Run pylint
         run: pylint evap tools
 
 

evap/contributor/tests/test_forms.py

@@ -67,7 +67,7 @@ def test_managers_only(self):
         )
         formset = InlineContributionFormset(instance=evaluation, form_kwargs={"evaluation": evaluation})
 
-        expected = set([questionnaire])
+        expected = {questionnaire}
         self.assertEqual(expected, set(formset.forms[0].fields["questionnaires"].queryset))
         self.assertEqual(expected, set(formset.forms[1].fields["questionnaires"].queryset))
 
@@ -79,7 +79,7 @@ def test_managers_only(self):
         )
         formset = InlineContributionFormset(instance=evaluation, form_kwargs={"evaluation": evaluation})
 
-        expected = set([questionnaire, questionnaire_managers_only])
+        expected = {questionnaire, questionnaire_managers_only}
         self.assertEqual(expected, set(formset.forms[0].fields["questionnaires"].queryset))
         self.assertEqual(expected, set(formset.forms[1].fields["questionnaires"].queryset))
 

evap/evaluation/auth.py

@@ -1,6 +1,6 @@
 import inspect
+from collections.abc import Callable
 from functools import wraps
-from typing import Callable
 
 from django.contrib.auth.backends import ModelBackend
 from django.core.exceptions import PermissionDenied
@@ -28,8 +28,7 @@ def authenticate(self, request, key):  # pylint: disable=arguments-differ
             return None
 
         try:
-            user = UserProfile.objects.get(login_key=key)
-            return user
+            return UserProfile.objects.get(login_key=key)
         except UserProfile.DoesNotExist:
             return None
 
@@ -144,12 +143,11 @@ def filter_users_by_claims(self, claims):
             return []
 
     def create_user(self, claims):
-        user = self.UserModel.objects.create(
+        return self.UserModel.objects.create(
             email=claims.get("email"),
             first_name_given=claims.get("given_name", ""),
             last_name=claims.get("family_name", ""),
         )
-        return user
 
     def update_user(self, user, claims):
         if not user.first_name_given:

evap/evaluation/management/commands/anonymize.py

@@ -94,8 +94,8 @@ def anonymize_users(self, first_names, last_names):
         if len(first_names) * len(last_names) < len(user_profiles) * 1.5:
             self.stdout.write(
                 "Warning: There are few example names compared to all that real data to be anonymized. "
-                + "Consider adding more data to the first_names.txt and last_names.txt files in the anonymize_data "
-                + "folder."
+                "Consider adding more data to the first_names.txt and last_names.txt files in the anonymize_data "
+                "folder."
             )
 
         while len(fake_usernames) < len(user_profiles):
@@ -111,7 +111,7 @@ def anonymize_users(self, first_names, last_names):
 
         # Actually replace all the real user data
         self.stdout.write("Replacing email addresses and login keys with fake ones...")
-        for user, name in zip(user_profiles, fake_usernames):
+        for user, name in zip(user_profiles, fake_usernames, strict=True):
             if user.email and user.email.split("@")[0] in Command.ignore_email_usernames:
                 continue
             user.first_name_given = name[0]
@@ -161,7 +161,7 @@ def anonymize_courses(self):
             # Shuffle public courses' names in order to decouple them from the results.
             # Also, assign public courses' names to private ones as their names may be confidential.
             self.stdout.write("Shuffling course names...")
-            public_names = list(set(map(lambda c: (c.name_de, c.name_en), public_courses)))
+            public_names = list({(c.name_de, c.name_en) for c in public_courses})
             random.shuffle(public_names)
 
             for i, course in enumerate(courses):
@@ -188,7 +188,7 @@ def anonymize_evaluations(self):
 
             self.stdout.write("Shuffling evaluation names...")
             named_evaluations = (evaluation for evaluation in evaluations if evaluation.name_de and evaluation.name_en)
-            names = list(set(map(lambda c: (c.name_de, c.name_en), named_evaluations)))
+            names = list({(c.name_de, c.name_en) for c in named_evaluations})
             random.shuffle(names)
 
             for i, evaluation in enumerate(evaluations):
@@ -244,7 +244,7 @@ def anonymize_answers(self, lorem_ipsum):
                 for question, counters in counters_per_question.items():
                     original_sum = sum(counter.count for counter in counters)
 
-                    missing_values = set(CHOICES[question.type].values).difference(set(c.answer for c in counters))
+                    missing_values = set(CHOICES[question.type].values).difference({c.answer for c in counters})
                     missing_values.discard(NO_ANSWER)  # don't add NO_ANSWER counter if it didn't exist before
                     for value in missing_values:
                         counters.append(
@@ -259,7 +259,7 @@ def anonymize_answers(self, lorem_ipsum):
                     index = random.randint(0, len(generated_counts) - 1)  # nosec
                     generated_counts[index] += to_add
 
-                    for counter, generated_count in zip(counters, generated_counts):
+                    for counter, generated_count in zip(counters, generated_counts, strict=True):
                         assert generated_count >= 0
                         counter.count = generated_count
 

evap/evaluation/management/commands/lint.py

@@ -9,4 +9,7 @@ class Command(BaseCommand):
     requires_migrations_checks = False
 
     def handle(self, *args, **options):
+        self.stdout.write("Executing ruff .")
+        subprocess.run(["ruff", "check", "."], check=False)  # nosec
+        self.stdout.write("Executing pylint evap")
         subprocess.run(["pylint", "evap", "tools"], check=False)  # nosec

evap/evaluation/management/commands/send_reminders.py

@@ -25,10 +25,7 @@ def get_sorted_evaluation_url_tuples_with_urgent_review() -> list[tuple[Evaluati
         for evaluation in Evaluation.objects.filter(state=Evaluation.State.EVALUATED)
         if evaluation.textanswer_review_state == Evaluation.TextAnswerReviewState.REVIEW_URGENT
     ]
-    evaluation_url_tuples = sorted(
-        evaluation_url_tuples, key=lambda evaluation_url_tuple: evaluation_url_tuple[0].full_name
-    )
-    return evaluation_url_tuples
+    return sorted(evaluation_url_tuples, key=lambda evaluation_url_tuple: evaluation_url_tuple[0].full_name)
 
 
 @log_exceptions
@@ -43,11 +40,10 @@ def handle(self, *args, **options):
 
     @staticmethod
     def send_student_reminders():
-        check_dates = []
-
-        # Collect end-dates of evaluations whose participants need to be reminded today.
-        for number_of_days in settings.REMIND_X_DAYS_AHEAD_OF_END_DATE:
-            check_dates.append(datetime.date.today() + datetime.timedelta(days=number_of_days))
+        check_dates = [
+            datetime.date.today() + datetime.timedelta(days=number_of_days)
+            for number_of_days in settings.REMIND_X_DAYS_AHEAD_OF_END_DATE
+        ]
 
         recipients = set()
         for evaluation in Evaluation.objects.filter(

evap/evaluation/migrations/0001_initial.py

@@ -257,7 +257,7 @@ class Migration(migrations.Migration):
         ),
         migrations.AlterUniqueTogether(
             name='course',
-            unique_together=set([('semester', 'degree', 'name_de'), ('semester', 'degree', 'name_en')]),
+            unique_together={('semester', 'degree', 'name_de'), ('semester', 'degree', 'name_en')},
         ),
         migrations.AddField(
             model_name='contribution',
@@ -273,6 +273,6 @@ class Migration(migrations.Migration):
         ),
         migrations.AlterUniqueTogether(
             name='contribution',
-            unique_together=set([('course', 'contributor')]),
+            unique_together={('course', 'contributor')},
         ),
     ]

evap/evaluation/migrations/0017_delete_old_degrees.py

@@ -14,7 +14,7 @@ class Migration(migrations.Migration):
         ),
         migrations.AlterUniqueTogether(
             name='course',
-            unique_together=set([('semester', 'name_en'), ('semester', 'name_de')]),
+            unique_together={('semester', 'name_en'), ('semester', 'name_de')},
         ),
         migrations.RemoveField(
             model_name='course',

evap/evaluation/migrations/0026_make_result_counters_unique.py

@@ -10,10 +10,10 @@ class Migration(migrations.Migration):
     operations = [
         migrations.AlterUniqueTogether(
             name='gradeanswercounter',
-            unique_together=set([('question', 'contribution', 'answer')]),
+            unique_together={('question', 'contribution', 'answer')},
         ),
         migrations.AlterUniqueTogether(
             name='likertanswercounter',
-            unique_together=set([('question', 'contribution', 'answer')]),
+            unique_together={('question', 'contribution', 'answer')},
         ),
     ]

evap/evaluation/migrations/0031_add_rating_answer_counter.py

@@ -25,6 +25,6 @@ class Migration(migrations.Migration):
         ),
         migrations.AlterUniqueTogether(
             name='ratinganswercounter',
-            unique_together=set([('question', 'contribution', 'answer')]),
+            unique_together={('question', 'contribution', 'answer')},
         ),
     ]

evap/evaluation/migrations/0033_remove_likert_and_grade_answer.py

@@ -10,7 +10,7 @@ class Migration(migrations.Migration):
     operations = [
         migrations.AlterUniqueTogether(
             name='gradeanswercounter',
-            unique_together=set([]),
+            unique_together=set(),
         ),
         migrations.RemoveField(
             model_name='gradeanswercounter',
@@ -22,7 +22,7 @@ class Migration(migrations.Migration):
         ),
         migrations.AlterUniqueTogether(
             name='likertanswercounter',
-            unique_together=set([]),
+            unique_together=set(),
         ),
         migrations.RemoveField(
             model_name='likertanswercounter',

evap/evaluation/migrations/0100_clear_evaluation_names.py

@@ -22,7 +22,7 @@ def name_evaluations(apps, _schema_editor):
             evaluation.name_en = course.name_en
             evaluation.save()
         else:
-            for i in range(0, course.evaluations.count()):
+            for i in range(course.evaluations.count()):
                 evaluation = Evaluation.objects.get(pk=course.evaluations.all()[i].pk)
                 evaluation.name_de = f"{course.name_de} ({i})"
                 evaluation.name_en = f"{course.name_en} ({i})"