From 9262be26055f6bd317f881d5bda81c262722c11f Mon Sep 17 00:00:00 2001 From: Richard Ebeling Date: Mon, 2 Oct 2023 21:14:57 +0200 Subject: [PATCH 01/54] Initial attempt at ruff --- .github/workflows/tests.yml | 5 ++++- pyproject.toml | 10 ++++++++++ requirements-dev.txt | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index be32408703..9b86ef8d14 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -88,9 +88,12 @@ jobs: - name: Setup python uses: ./.github/setup_python - - name: Run linter + - name: Run pylint run: pylint evap -j 0 + - name: Run ruff + run: ruff . + formatter: runs-on: ubuntu-22.04 diff --git a/pyproject.toml b/pyproject.toml index 137717643f..2204f9bfc1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,6 +22,16 @@ extend_skip_glob = ["**/migrations/*"] ############################################## +[tool.ruff] +select = ["F", "E", "B", "W", "N", "UP", "YTT"] +ignore = [ + "E501", # line-too-long: black does code formatting for us +] + + +############################################## + + [tool.pylint.master] jobs = 0 diff --git a/requirements-dev.txt b/requirements-dev.txt index 4580c18f00..d126e0d916 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -10,5 +10,6 @@ mypy~=1.7.0 openpyxl-stubs~=0.1.25 pylint-django~=2.5.4 pylint~=3.0.1 +ruff==0.0.291 tblib~=3.0.0 xlrd~=2.0.1 From 94a5a62fa5cbb2c9cd618f6326b5236e678cab68 Mon Sep 17 00:00:00 2001 From: Richard Ebeling Date: Mon, 9 Oct 2023 21:21:55 +0200 Subject: [PATCH 02/54] Progress --- pyproject.toml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 2204f9bfc1..d5a33d7ef3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,11 +23,20 @@ extend_skip_glob = ["**/migrations/*"] ############################################## [tool.ruff] +# We'd like to enable N806, but we'd need to allow some special cases that we currently can't: +# * migrations have model classes as local variables, we use PascalCase for these +# * custom assert methods use camelCase +# * Formsets use PascalCase + select = ["F", "E", "B", "W", "N", "UP", "YTT"] ignore = [ "E501", # line-too-long: black does code formatting for us + "N806", ] +ignore-init-module-imports = true +pep8-naming.extend-ignore-names = ["assert*", "*Formset"] + ############################################## From 8a56e8ba449a2eab46abf80ab43938035aa01697 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 23 Oct 2023 20:20:29 +0200 Subject: [PATCH 03/54] prohibit FIXME, TODO and XXX --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d5a33d7ef3..26c79cee8b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,10 +28,11 @@ extend_skip_glob = ["**/migrations/*"] # * custom assert methods use camelCase # * Formsets use PascalCase -select = ["F", "E", "B", "W", "N", "UP", "YTT"] +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX"] ignore = [ "E501", # line-too-long: black does code formatting for us "N806", + "FIX004", # hacks should be possible ] ignore-init-module-imports = true From ab8b453713be92c095b165142447ea7692bbb065 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 23 Oct 2023 20:57:26 +0200 Subject: [PATCH 04/54] add zip strict parameter --- evap/evaluation/management/commands/anonymize.py | 4 ++-- evap/evaluation/templatetags/evaluation_filters.py | 4 ++-- evap/results/exporters.py | 8 +++++--- evap/results/tests/test_exporters.py | 2 +- evap/results/tools.py | 6 ++++-- evap/results/views.py | 2 +- evap/staff/tests/test_views.py | 2 +- evap/student/forms.py | 2 +- 8 files changed, 17 insertions(+), 13 deletions(-) diff --git a/evap/evaluation/management/commands/anonymize.py b/evap/evaluation/management/commands/anonymize.py index 272bc9f2e2..71b68d2952 100644 --- a/evap/evaluation/management/commands/anonymize.py +++ b/evap/evaluation/management/commands/anonymize.py @@ -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] @@ -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 diff --git a/evap/evaluation/templatetags/evaluation_filters.py b/evap/evaluation/templatetags/evaluation_filters.py index bc6b7dabf0..4a8119a4ac 100644 --- a/evap/evaluation/templatetags/evaluation_filters.py +++ b/evap/evaluation/templatetags/evaluation_filters.py @@ -76,12 +76,12 @@ @register.filter(name="zip") def _zip(a, b): - return zip(a, b) + return zip(a, b, strict=False) @register.filter() def zip_choices(counts, choices): - return zip(counts, choices.names, choices.colors, choices.values) + return zip(counts, choices.names, choices.colors, choices.values, strict=False) @register.filter diff --git a/evap/results/exporters.py b/evap/results/exporters.py index 2cce248693..68f89d7b57 100644 --- a/evap/results/exporters.py +++ b/evap/results/exporters.py @@ -223,11 +223,13 @@ def write_overall_results(self, evaluations_with_results, course_results_exist): ) self.write_cell(_("Evaluation weight"), "bold") - weight_percentages = (f"{e.weight_percentage}%" if gt1 else None for e, gt1 in zip(evaluations, count_gt_1)) + weight_percentages = ( + f"{e.weight_percentage}%" if gt1 else None for e, gt1 in zip(evaluations, count_gt_1, strict=True) + ) self.write_row(weight_percentages, lambda s: "evaluation_weight" if s is not None else "default") self.write_cell(_("Course Grade"), "bold") - for evaluation, gt1 in zip(evaluations, count_gt_1): + for evaluation, gt1 in zip(evaluations, count_gt_1, strict=True): if not gt1: self.write_cell() continue @@ -368,5 +370,5 @@ def export_impl(self): # pylint: disable=arguments-differ question_title = (f"{contributor_name}: " if contributor_name else "") + question.text first_col = chain([question_title], repeat("")) - for answer, first_cell, line_style in zip(answers, first_col, line_styles): + for answer, first_cell, line_style in zip(answers, first_col, line_styles, strict=False): self.write_row([first_cell, answer], style=line_style) diff --git a/evap/results/tests/test_exporters.py b/evap/results/tests/test_exporters.py index c5779ea3bb..f5498eb7dc 100644 --- a/evap/results/tests/test_exporters.py +++ b/evap/results/tests/test_exporters.py @@ -449,7 +449,7 @@ def test_course_grade(self): questionnaire = baker.make(Questionnaire) question = baker.make(Question, type=QuestionType.POSITIVE_LIKERT, questionnaire=questionnaire) - for grades, e in zip(grades_per_eval, evaluations): + for grades, e in zip(grades_per_eval, evaluations, strict=True): make_rating_answer_counters(question, e.general_contribution, grades) e.general_contribution.questionnaires.set([questionnaire]) for evaluation in evaluations: diff --git a/evap/results/tools.py b/evap/results/tools.py index fceefa3bf4..59480117d6 100644 --- a/evap/results/tools.py +++ b/evap/results/tools.py @@ -82,7 +82,9 @@ def approval_count(self) -> int | None: def average(self) -> float | None: if not self.has_answers: return None - return sum(grade * count for count, grade in zip(self.counts, self.choices.grades)) / self.count_sum + return ( + sum(grade * count for count, grade in zip(self.counts, self.choices.grades, strict=True)) / self.count_sum + ) @property def has_answers(self) -> bool: @@ -275,7 +277,7 @@ def unipolarized_distribution(result): if not result.counts: return None - for counts, grade in zip(result.counts, result.choices.grades): + for counts, grade in zip(result.counts, result.choices.grades, strict=True): grade_fraction, grade = modf(grade) grade = int(grade) summed_distribution[grade - 1] += (1 - grade_fraction) * counts diff --git a/evap/results/views.py b/evap/results/views.py index f1e2a7913d..92caa09de0 100644 --- a/evap/results/views.py +++ b/evap/results/views.py @@ -147,7 +147,7 @@ def index(request): annotated_courses = ( Course.objects.filter(pk__in=course_pks).annotate(num_evaluations=Count("evaluations")).order_by("pk").defer() ) - for course, annotated_course in zip(courses_and_evaluations.keys(), annotated_courses): + for course, annotated_course in zip(courses_and_evaluations.keys(), annotated_courses, strict=True): course.num_evaluations = annotated_course.num_evaluations degrees = Degree.objects.filter(courses__pk__in=course_pks).distinct() diff --git a/evap/staff/tests/test_views.py b/evap/staff/tests/test_views.py index 4e126ac97e..cbd6569367 100644 --- a/evap/staff/tests/test_views.py +++ b/evap/staff/tests/test_views.py @@ -2703,7 +2703,7 @@ def test_suggested_evaluation_ordering(self): _quantity=2, ) - for evaluation, answer_count in zip(evaluations, [1, 2]): + for evaluation, answer_count in zip(evaluations, [1, 2], strict=True): contribution = baker.make(Contribution, evaluation=evaluation, _fill_optional=["contributor"]) baker.make(TextAnswer, contribution=contribution, question__type=QuestionType.TEXT, _quantity=answer_count) diff --git a/evap/student/forms.py b/evap/student/forms.py index 459668c175..dccd9f6237 100644 --- a/evap/student/forms.py +++ b/evap/student/forms.py @@ -60,7 +60,7 @@ def deconstruct(self): def from_question(cls, question: Question): return cls( widget_choices=CHOICES[question.type], - choices=zip(CHOICES[question.type].values, CHOICES[question.type].names), + choices=zip(CHOICES[question.type].values, CHOICES[question.type].names, strict=True), label=question.text, allows_textanswer=question.allows_additional_textanswers, ) From c911e90a4cb73a3dfb0f6b109fff4e928be3ceef Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 23 Oct 2023 20:58:44 +0200 Subject: [PATCH 05/54] import Callable from collections.abc --- evap/evaluation/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/evaluation/auth.py b/evap/evaluation/auth.py index b71a8dea46..e5565b81e5 100644 --- a/evap/evaluation/auth.py +++ b/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 From 5b46b3d1d68e2a0582efaa0ce0e0ffc2ddb5e3a3 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 23 Oct 2023 21:11:04 +0200 Subject: [PATCH 06/54] add ignores in settings --- evap/settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/evap/settings.py b/evap/settings.py index 96cdd9d0b4..535e512c7c 100644 --- a/evap/settings.py +++ b/evap/settings.py @@ -375,7 +375,7 @@ class ManifestStaticFilesStorageWithJsReplacement(ManifestStaticFilesStorage): ### Allowed chosen first names / display names -def CHARACTER_ALLOWED_IN_NAME(character): # pylint: disable=invalid-name +def CHARACTER_ALLOWED_IN_NAME(character): # pylint: disable=invalid-name # noqa: N802 return any( ( ord(character) in range(32, 127), # printable ASCII / Basic Latin characters @@ -412,7 +412,7 @@ def CHARACTER_ALLOWED_IN_NAME(character): # pylint: disable=invalid-name # pylint: disable=unused-wildcard-import,wildcard-import,useless-suppression # the import can overwrite locals with a slightly different type (e.g. DATABASES), which is fine. - from evap.localsettings import * # type: ignore + from evap.localsettings import * # type: ignore # noqa: F403 except ImportError: pass From ee949d0bd9afee750c42c5000dd44eed57def463 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 23 Oct 2023 22:08:47 +0200 Subject: [PATCH 07/54] add __all__ --- evap/staff/importers/__init__.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/evap/staff/importers/__init__.py b/evap/staff/importers/__init__.py index bc58f53589..dac77ea487 100644 --- a/evap/staff/importers/__init__.py +++ b/evap/staff/importers/__init__.py @@ -2,3 +2,12 @@ from .enrollment import import_enrollments from .person import import_persons_from_evaluation, import_persons_from_file from .user import import_users + +__all__ = [ + "ImporterLog", + "ImporterLogEntry", + "import_enrollments", + "import_persons_from_evaluation", + "import_persons_from_file", + "import_users", +] From d47105b123a666d9bf4b8bb2a276e42db72a36c1 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 23 Oct 2023 22:09:08 +0200 Subject: [PATCH 08/54] add stacklevel to warning, when top level code is executed twice --- evap/results/exporters.py | 1 + 1 file changed, 1 insertion(+) diff --git a/evap/results/exporters.py b/evap/results/exporters.py index 68f89d7b57..6c191af8ca 100644 --- a/evap/results/exporters.py +++ b/evap/results/exporters.py @@ -71,6 +71,7 @@ def init_grade_styles(cls): "This can happen, if the file is imported / run multiple " "times in one application run.", ImportWarning, + stacklevel=2, ) return From 6db57f32eb1fddfa222c4feeb638a229153935fb Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 23 Oct 2023 22:19:35 +0200 Subject: [PATCH 09/54] add ruff ignores to pylint ignores --- evap/evaluation/tests/test_auth.py | 2 +- evap/locale/de/formats.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/evap/evaluation/tests/test_auth.py b/evap/evaluation/tests/test_auth.py index 899b6e4753..a49d14b612 100644 --- a/evap/evaluation/tests/test_auth.py +++ b/evap/evaluation/tests/test_auth.py @@ -187,7 +187,7 @@ class TestAuthDecorators(WebTest): def setUpTestData(cls): @class_or_function_check_decorator def check_decorator(user: UserProfile) -> bool: - return getattr(user, "some_condition") # mocked later + return getattr(user, "some_condition") # noqa: B009 # mocked later @check_decorator def function_based_view(_request): diff --git a/evap/locale/de/formats.py b/evap/locale/de/formats.py index 42ad0f8e57..a847972cae 100644 --- a/evap/locale/de/formats.py +++ b/evap/locale/de/formats.py @@ -1 +1 @@ -from evap.locale.en.formats import * # pylint: disable=unused-wildcard-import,wildcard-import +from evap.locale.en.formats import * # pylint: disable=unused-wildcard-import,wildcard-import # noqa: F403 From 39662a33d046f09b32c3ac0070bf817ba7414d3a Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 30 Oct 2023 22:54:28 +0100 Subject: [PATCH 10/54] add some more rules --- pyproject.toml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 26c79cee8b..b0ce858771 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,16 +28,20 @@ extend_skip_glob = ["**/migrations/*"] # * custom assert methods use camelCase # * Formsets use PascalCase -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX"] +target-version = "py310" +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A"] ignore = [ "E501", # line-too-long: black does code formatting for us - "N806", "FIX004", # hacks should be possible + "A003", ] ignore-init-module-imports = true pep8-naming.extend-ignore-names = ["assert*", "*Formset"] +[tool.ruff.per-file-ignores] +"**/migrations/*.py" = ["N806"] + ############################################## From dee96bfb9a0f5ffe7fd1f7c5fe4bcdf6db9a314e Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 30 Oct 2023 22:55:25 +0100 Subject: [PATCH 11/54] add DJ checks --- evap/evaluation/models.py | 16 ++++++++-------- evap/evaluation/models_logging.py | 26 +++++++++++++------------- evap/staff/importers/base.py | 4 ++-- pyproject.toml | 3 ++- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 7fa589026d..f528df8999 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -194,10 +194,6 @@ class Visibility(models.IntegerChoices): objects = QuestionnaireManager() - def clean(self): - if self.type == self.Type.CONTRIBUTOR and self.is_locked: - raise ValidationError({"is_locked": _("Contributor questionnaires cannot be locked.")}) - class Meta: ordering = ["type", "order", "pk"] verbose_name = _("questionnaire") @@ -206,6 +202,10 @@ class Meta: def __str__(self): return self.name + def clean(self): + if self.type == self.Type.CONTRIBUTOR and self.is_locked: + raise ValidationError({"is_locked": _("Contributor questionnaires cannot be locked.")}) + def __lt__(self, other): return (self.type, self.order, self.pk) < (other.type, other.order, other.pk) @@ -1470,7 +1470,7 @@ class TextAnswer(Answer): answer = models.TextField(verbose_name=_("answer")) # If the text answer was changed during review, original_answer holds the original text. Otherwise, it's null. - original_answer = models.TextField(verbose_name=_("original answer"), blank=True, null=True) + original_answer = models.TextField(verbose_name=_("original answer"), blank=True, null=True) # noqa: DJ001 class ReviewDecision(models.TextChoices): """ @@ -1613,9 +1613,6 @@ class Infotext(models.Model): content_en = models.TextField(verbose_name=_("content (english)"), blank=True) content = translate(en="content_en", de="content_de") - def is_empty(self): - return not (self.title or self.content) - class Page(models.TextChoices): STUDENT_INDEX = ("student_index", "Student index page") CONTRIBUTOR_INDEX = ("contributor_index", "Contributor index page") @@ -1640,6 +1637,9 @@ class Meta: ), ) + def is_empty(self): + return not (self.title or self.content) + class UserProfileManager(BaseUserManager): def create_user(self, *, email, password=None, first_name=None, last_name=None): diff --git a/evap/evaluation/models_logging.py b/evap/evaluation/models_logging.py index f9cd00332c..e54ebb0f47 100644 --- a/evap/evaluation/models_logging.py +++ b/evap/evaluation/models_logging.py @@ -128,6 +128,19 @@ class LoggedModel(models.Model): class Meta: abstract = True + def save(self, *args, **kw): + # Are we creating a new instance? + # https://docs.djangoproject.com/en/3.0/ref/models/instances/#customizing-model-loading + if self._state.adding: + # we need to attach a logentry to an existing object, so we save this newly created instance first + super().save(*args, **kw) + self.log_instance_create() + else: + # when saving an existing instance, we get changes by comparing to the version from the database + # therefore we save the instance after building the logentry + self.log_instance_change() + super().save(*args, **kw) + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._logentry = None @@ -239,19 +252,6 @@ def _update_log(self, changes, action_type: InstanceActionType, store_in_db=True if store_in_db: self._logentry.save() - def save(self, *args, **kw): - # Are we creating a new instance? - # https://docs.djangoproject.com/en/3.0/ref/models/instances/#customizing-model-loading - if self._state.adding: - # we need to attach a logentry to an existing object, so we save this newly created instance first - super().save(*args, **kw) - self.log_instance_create() - else: - # when saving an existing instance, we get changes by comparing to the version from the database - # therefore we save the instance after building the logentry - self.log_instance_change() - super().save(*args, **kw) - def delete(self, *args, **kw): self.log_instance_delete() self.related_logentries().delete() diff --git a/evap/staff/importers/base.py b/evap/staff/importers/base.py index 2c942ec7a9..7a9ba97f3a 100644 --- a/evap/staff/importers/base.py +++ b/evap/staff/importers/base.py @@ -179,9 +179,9 @@ class InputRow(ABC): # MyPy is currently broken with abstract properties: https://github.com/python/mypy/issues/8996 column_count: int + @abstractmethod def __init__(self, location: ExcelFileLocation, *cells: Iterable[str]): - # this can be an abstractmethod from python3.10 on, before it doesn't work with the derived dataclasses - raise NotImplementedError # pragma: no cover + pass @classmethod def from_cells(cls, location: ExcelFileLocation, cells: Iterable[str]): diff --git a/pyproject.toml b/pyproject.toml index b0ce858771..485d8e31cc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,11 +29,12 @@ extend_skip_glob = ["**/migrations/*"] # * Formsets use PascalCase target-version = "py310" -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A"] +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ"] ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible "A003", + "DJ008", # do not use __str__ where it is not required ] ignore-init-module-imports = true From c747b286e27f48c7529461187c120bf83f78af07 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 19:31:20 +0100 Subject: [PATCH 12/54] improve comments (fixup!) --- pyproject.toml | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 485d8e31cc..e092365ac2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,25 +23,20 @@ extend_skip_glob = ["**/migrations/*"] ############################################## [tool.ruff] -# We'd like to enable N806, but we'd need to allow some special cases that we currently can't: -# * migrations have model classes as local variables, we use PascalCase for these -# * custom assert methods use camelCase -# * Formsets use PascalCase - target-version = "py310" select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ"] ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible - "A003", + "A003", # we shadow some builtins sometime "DJ008", # do not use __str__ where it is not required ] ignore-init-module-imports = true -pep8-naming.extend-ignore-names = ["assert*", "*Formset"] +pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert methods use camelCase; Formsets use PascalCase [tool.ruff.per-file-ignores] -"**/migrations/*.py" = ["N806"] +"**/migrations/*.py" = ["N806"] # migrations have model classes as local variables, we use PascalCase for these ############################################## From fdcbe48a3d0064ea984dd9b4395c22795880d08e Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 19:31:37 +0100 Subject: [PATCH 13/54] ruff formatter --- pyproject.toml | 4 ++++ requirements-dev.txt | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index e092365ac2..05682f8be5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,6 +24,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" +line-length = 120 select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ"] ignore = [ "E501", # line-too-long: black does code formatting for us @@ -38,6 +39,9 @@ pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert metho [tool.ruff.per-file-ignores] "**/migrations/*.py" = ["N806"] # migrations have model classes as local variables, we use PascalCase for these +"**/commands/*.py" = ["T201"] # printing in commands is ok +[tool.ruff.format] +exclude = ["**/urls.py", "**/migrations/*.py"] ############################################## diff --git a/requirements-dev.txt b/requirements-dev.txt index d126e0d916..283595d57e 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -10,6 +10,6 @@ mypy~=1.7.0 openpyxl-stubs~=0.1.25 pylint-django~=2.5.4 pylint~=3.0.1 -ruff==0.0.291 +ruff==0.1.5 tblib~=3.0.0 xlrd~=2.0.1 From aa6d58ed992323a4b84ad7e154a8e662164b01cf Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 19:32:01 +0100 Subject: [PATCH 14/54] enable "free" tests --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 05682f8be5..7e0ae4b5db 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ"] +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT"] # COM,C4,ISC,INP,PIE,PYI,Q,RSE,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible From 5dd6441d6931973172510bfb163aa26855a924fc Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 19:49:37 +0100 Subject: [PATCH 15/54] replace pylint disables with ruff --- evap/evaluation/models.py | 2 +- evap/locale/de/formats.py | 2 +- evap/settings.py | 6 ++---- pyproject.toml | 1 + 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index f528df8999..e3077fdf2f 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -444,7 +444,7 @@ class State: ) class TextAnswerReviewState(Enum): - do_not_call_in_templates = True # pylint: disable=invalid-name + do_not_call_in_templates = True NO_TEXTANSWERS = auto() NO_REVIEW_NEEDED = auto() REVIEW_NEEDED = auto() diff --git a/evap/locale/de/formats.py b/evap/locale/de/formats.py index a847972cae..8a74b1ce39 100644 --- a/evap/locale/de/formats.py +++ b/evap/locale/de/formats.py @@ -1 +1 @@ -from evap.locale.en.formats import * # pylint: disable=unused-wildcard-import,wildcard-import # noqa: F403 +from evap.locale.en.formats import * # noqa: F403 diff --git a/evap/settings.py b/evap/settings.py index 535e512c7c..30be729665 100644 --- a/evap/settings.py +++ b/evap/settings.py @@ -375,7 +375,7 @@ class ManifestStaticFilesStorageWithJsReplacement(ManifestStaticFilesStorage): ### Allowed chosen first names / display names -def CHARACTER_ALLOWED_IN_NAME(character): # pylint: disable=invalid-name # noqa: N802 +def CHARACTER_ALLOWED_IN_NAME(character): # noqa: N802 return any( ( ord(character) in range(32, 127), # printable ASCII / Basic Latin characters @@ -407,9 +407,7 @@ def CHARACTER_ALLOWED_IN_NAME(character): # pylint: disable=invalid-name # noq # Create a localsettings.py if you want to locally override settings # and don't want the changes to appear in 'git status'. try: - # if a localsettings file exists (vagrant), this will cause wildcard-import errors - # if it does not, (GitHub), it would cause useless-suppression - # pylint: disable=unused-wildcard-import,wildcard-import,useless-suppression + # localsettings file may exist (vagrant) or do not (GitHub) # the import can overwrite locals with a slightly different type (e.g. DATABASES), which is fine. from evap.localsettings import * # type: ignore # noqa: F403 diff --git a/pyproject.toml b/pyproject.toml index 7e0ae4b5db..13f9d154c2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -91,6 +91,7 @@ disable = [ "unsupported-binary-operation", # broken in pylint: https://github.com/PyCQA/pylint/issues/7381 "use-implicit-booleaness-not-comparison-to-string", # forces us to use less expressive code "use-implicit-booleaness-not-comparison-to-zero", # forces us to use less expressive code + "unused-wildcard-import", "wildcard-import", "invalid-name", # covered by ruff ] ############################################## From 037041c3261d3c0d4d37ad67b24326114b5e1b08 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 21:26:08 +0100 Subject: [PATCH 16/54] enable and (auto) fix set|list|map literal --- evap/contributor/tests/test_forms.py | 4 ++-- evap/evaluation/management/commands/anonymize.py | 6 +++--- evap/evaluation/migrations/0001_initial.py | 4 ++-- evap/evaluation/migrations/0017_delete_old_degrees.py | 2 +- .../migrations/0026_make_result_counters_unique.py | 4 ++-- .../migrations/0031_add_rating_answer_counter.py | 2 +- .../migrations/0033_remove_likert_and_grade_answer.py | 4 ++-- .../migrations/0005_make_description_unique_per_course.py | 2 +- .../migrations/0008_add_gradedocument_description_en.py | 2 +- .../0010_gradedocument_description_en_add_unique.py | 2 +- evap/results/tests/test_exporters.py | 2 +- evap/staff/forms.py | 2 +- evap/staff/tests/test_importers.py | 4 ++-- evap/staff/tests/test_tools.py | 4 ++-- evap/staff/tests/test_views.py | 8 ++++---- pyproject.toml | 2 +- 16 files changed, 27 insertions(+), 27 deletions(-) diff --git a/evap/contributor/tests/test_forms.py b/evap/contributor/tests/test_forms.py index 900d13ef40..8dc7a58591 100644 --- a/evap/contributor/tests/test_forms.py +++ b/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)) diff --git a/evap/evaluation/management/commands/anonymize.py b/evap/evaluation/management/commands/anonymize.py index 71b68d2952..ad017c9e44 100644 --- a/evap/evaluation/management/commands/anonymize.py +++ b/evap/evaluation/management/commands/anonymize.py @@ -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( diff --git a/evap/evaluation/migrations/0001_initial.py b/evap/evaluation/migrations/0001_initial.py index 9c7ff500a6..f439af90b0 100644 --- a/evap/evaluation/migrations/0001_initial.py +++ b/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')}, ), ] diff --git a/evap/evaluation/migrations/0017_delete_old_degrees.py b/evap/evaluation/migrations/0017_delete_old_degrees.py index 53633687a8..756cb52771 100644 --- a/evap/evaluation/migrations/0017_delete_old_degrees.py +++ b/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', diff --git a/evap/evaluation/migrations/0026_make_result_counters_unique.py b/evap/evaluation/migrations/0026_make_result_counters_unique.py index 2d1c894167..0b2db2a540 100644 --- a/evap/evaluation/migrations/0026_make_result_counters_unique.py +++ b/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')}, ), ] diff --git a/evap/evaluation/migrations/0031_add_rating_answer_counter.py b/evap/evaluation/migrations/0031_add_rating_answer_counter.py index 7f3559a052..1c507d0f8a 100644 --- a/evap/evaluation/migrations/0031_add_rating_answer_counter.py +++ b/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')}, ), ] diff --git a/evap/evaluation/migrations/0033_remove_likert_and_grade_answer.py b/evap/evaluation/migrations/0033_remove_likert_and_grade_answer.py index 53e704e674..c355e740a8 100644 --- a/evap/evaluation/migrations/0033_remove_likert_and_grade_answer.py +++ b/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', diff --git a/evap/grades/migrations/0005_make_description_unique_per_course.py b/evap/grades/migrations/0005_make_description_unique_per_course.py index 06227dc0ff..b76d14c20d 100644 --- a/evap/grades/migrations/0005_make_description_unique_per_course.py +++ b/evap/grades/migrations/0005_make_description_unique_per_course.py @@ -10,6 +10,6 @@ class Migration(migrations.Migration): operations = [ migrations.AlterUniqueTogether( name='gradedocument', - unique_together=set([('course', 'description')]), + unique_together={('course', 'description')}, ), ] diff --git a/evap/grades/migrations/0008_add_gradedocument_description_en.py b/evap/grades/migrations/0008_add_gradedocument_description_en.py index 385e1a39da..301df59895 100644 --- a/evap/grades/migrations/0008_add_gradedocument_description_en.py +++ b/evap/grades/migrations/0008_add_gradedocument_description_en.py @@ -17,7 +17,7 @@ class Migration(migrations.Migration): ), migrations.AlterUniqueTogether( name='gradedocument', - unique_together=set([('course', 'description_de')]), + unique_together={('course', 'description_de')}, ), migrations.AlterField( model_name='gradedocument', diff --git a/evap/grades/migrations/0010_gradedocument_description_en_add_unique.py b/evap/grades/migrations/0010_gradedocument_description_en_add_unique.py index f7e084176b..8d99b54522 100644 --- a/evap/grades/migrations/0010_gradedocument_description_en_add_unique.py +++ b/evap/grades/migrations/0010_gradedocument_description_en_add_unique.py @@ -12,6 +12,6 @@ class Migration(migrations.Migration): operations = [ migrations.AlterUniqueTogether( name='gradedocument', - unique_together=set([('course', 'description_de'), ('course', 'description_en')]), + unique_together={('course', 'description_de'), ('course', 'description_en')}, ), ] diff --git a/evap/results/tests/test_exporters.py b/evap/results/tests/test_exporters.py index f5498eb7dc..0490769a82 100644 --- a/evap/results/tests/test_exporters.py +++ b/evap/results/tests/test_exporters.py @@ -399,7 +399,7 @@ def test_multiple_evaluations(self): sheet = self.get_export_sheet(semester, degree, [evaluation1.course.type.id, evaluation2.course.type.id]) self.assertEqual( - set(sheet.row_values(0)[1:]), set((evaluation1.full_name + "\n", evaluation2.full_name + "\n")) + set(sheet.row_values(0)[1:]), {evaluation1.full_name + "\n", evaluation2.full_name + "\n"} ) def test_correct_grades_and_bottom_numbers(self): diff --git a/evap/staff/forms.py b/evap/staff/forms.py index 810feeda6a..0889de64a5 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -683,7 +683,7 @@ def email_addresses(self): recipients = self.template.recipient_list_for_evaluation( self.evaluation, self.recipient_groups, filter_users_in_cc=False ) - return set(user.email for user in recipients if user.email) + return {user.email for user in recipients if user.email} def send(self, request): self.template.subject = self.cleaned_data.get("subject") diff --git a/evap/staff/tests/test_importers.py b/evap/staff/tests/test_importers.py index 25e5e7a66f..e2ec089d65 100644 --- a/evap/staff/tests/test_importers.py +++ b/evap/staff/tests/test_importers.py @@ -983,7 +983,7 @@ def test_import_new_contributor(self): self.assertEqual(self.evaluation1.contributions.count(), 3) self.assertEqual( set(UserProfile.objects.filter(contributions__evaluation=self.evaluation1)), - set([self.contributor1, self.contributor2]), + {self.contributor1, self.contributor2}, ) def test_import_existing_participant(self): @@ -1032,7 +1032,7 @@ def test_import_new_participant(self): self.assertIn(f"{self.participant2.email}", "".join(success_messages)) self.assertEqual(self.evaluation1.participants.count(), 2) - self.assertEqual(set(self.evaluation1.participants.all()), set([self.participant1, self.participant2])) + self.assertEqual(set(self.evaluation1.participants.all()), {self.participant1, self.participant2}) def test_imported_participants_are_made_active(self): self.participant2.is_active = False diff --git a/evap/staff/tests/test_tools.py b/evap/staff/tests/test_tools.py index 306cfa89b7..0a65d10e4d 100644 --- a/evap/staff/tests/test_tools.py +++ b/evap/staff/tests/test_tools.py @@ -79,11 +79,11 @@ def test_merge_handles_all_attributes(self): user1 = baker.make(UserProfile) user2 = baker.make(UserProfile) - all_attrs = list(field.name for field in UserProfile._meta.get_fields(include_hidden=True)) + all_attrs = [field.name for field in UserProfile._meta.get_fields(include_hidden=True)] # these are relations to intermediate models generated by django for m2m relations. # we can safely ignore these since the "normal" fields of the m2m relations are present as well. - all_attrs = list(attr for attr in all_attrs if not attr.startswith("UserProfile_")) + all_attrs = [attr for attr in all_attrs if not attr.startswith("UserProfile_")] # equally named fields are not supported, sorry self.assertEqual(len(all_attrs), len(set(all_attrs))) diff --git a/evap/staff/tests/test_views.py b/evap/staff/tests/test_views.py index cbd6569367..ab6076122b 100644 --- a/evap/staff/tests/test_views.py +++ b/evap/staff/tests/test_views.py @@ -3641,16 +3641,16 @@ def test_questionnaire_assignment(self): self.assertEqual( set(self.evaluation_1.general_contribution.questionnaires.all()), - set([self.questionnaire_1, self.questionnaire_2]), + {self.questionnaire_1, self.questionnaire_2}, ) - self.assertEqual(set(self.evaluation_2.general_contribution.questionnaires.all()), set([self.questionnaire_2])) + self.assertEqual(set(self.evaluation_2.general_contribution.questionnaires.all()), {self.questionnaire_2}) self.assertEqual( set(self.evaluation_1.contributions.get(contributor=self.responsible).questionnaires.all()), - set([self.questionnaire_responsible]), + {self.questionnaire_responsible}, ) self.assertEqual( set(self.evaluation_2.contributions.get(contributor=self.responsible).questionnaires.all()), - set([self.questionnaire_responsible]), + {self.questionnaire_responsible}, ) diff --git a/pyproject.toml b/pyproject.toml index 13f9d154c2..1cabd4615b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT"] # COM,C4,ISC,INP,PIE,PYI,Q,RSE,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4"] # COM,ISC,INP,PIE,PYI,Q,RSE,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible From fbfa113de3a2e2aab25ee28cda692d1d50d12ba5 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 21:28:13 +0100 Subject: [PATCH 17/54] enable and (auto) fix implicit string concat --- .../management/commands/anonymize.py | 4 ++-- evap/results/tests/test_exporters.py | 4 +--- evap/staff/importers/enrollment.py | 2 +- evap/staff/tests/test_importers.py | 24 +++++++++---------- evap/staff/tests/test_views.py | 4 ++-- pyproject.toml | 2 +- 6 files changed, 19 insertions(+), 21 deletions(-) diff --git a/evap/evaluation/management/commands/anonymize.py b/evap/evaluation/management/commands/anonymize.py index ad017c9e44..fc248989eb 100644 --- a/evap/evaluation/management/commands/anonymize.py +++ b/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): diff --git a/evap/results/tests/test_exporters.py b/evap/results/tests/test_exporters.py index 0490769a82..5e6dad0230 100644 --- a/evap/results/tests/test_exporters.py +++ b/evap/results/tests/test_exporters.py @@ -398,9 +398,7 @@ def test_multiple_evaluations(self): sheet = self.get_export_sheet(semester, degree, [evaluation1.course.type.id, evaluation2.course.type.id]) - self.assertEqual( - set(sheet.row_values(0)[1:]), {evaluation1.full_name + "\n", evaluation2.full_name + "\n"} - ) + self.assertEqual(set(sheet.row_values(0)[1:]), {evaluation1.full_name + "\n", evaluation2.full_name + "\n"}) def test_correct_grades_and_bottom_numbers(self): degree = baker.make(Degree) diff --git a/evap/staff/importers/enrollment.py b/evap/staff/importers/enrollment.py index 11ae291887..c6ad63184c 100644 --- a/evap/staff/importers/enrollment.py +++ b/evap/staff/importers/enrollment.py @@ -438,7 +438,7 @@ def finalize(self) -> None: self.importer_log.add_warning( _( 'Course "{course_name}" already exists. The course will not be created, instead users are imported into the ' - + "evaluation of the existing course and any additional degrees are added." + "evaluation of the existing course and any additional degrees are added.", ).format(course_name=name_en), category=ImporterLogEntry.Category.EXISTS, ) diff --git a/evap/staff/tests/test_importers.py b/evap/staff/tests/test_importers.py index e2ec089d65..e9330c61b7 100644 --- a/evap/staff/tests/test_importers.py +++ b/evap/staff/tests/test_importers.py @@ -665,7 +665,7 @@ def test_existing_course_is_not_recreated(self): self.assertIn( 'Course "Shake" already exists. The course will not be created, instead users are imported into the ' - + "evaluation of the existing course and any additional degrees are added.", + "evaluation of the existing course and any additional degrees are added.", warnings_test, ) self.assertListEqual(warnings_test, warnings_notest) @@ -716,9 +716,9 @@ def test_existing_course_different_attributes(self): importer_log, ImporterLogEntry.Category.COURSE, "Sheet "BA Belegungen", row 2 and 1 other place: Course "Shake" already exists in this " - + "semester, but the courses can not be merged for the following reasons:" - + "
- the course type does not match" - + "
- the responsibles of the course do not match", + "semester, but the courses can not be merged for the following reasons:" + "
- the course type does not match" + "
- the responsibles of the course do not match", ) def test_existing_course_with_published_evaluation(self): @@ -737,8 +737,8 @@ def test_existing_course_with_published_evaluation(self): importer_log, ImporterLogEntry.Category.COURSE, "Sheet "BA Belegungen", row 2 and 1 other place: " - + "Course "Shake" already exists in this semester, but the courses can not be merged for the following reasons:
" - + "- the import would add participants to the existing evaluation but the evaluation is already running", + "Course "Shake" already exists in this semester, but the courses can not be merged for the following reasons:
" + "- the import would add participants to the existing evaluation but the evaluation is already running", ) # Attempt with earlier state but set _participant_count @@ -772,8 +772,8 @@ def test_existing_course_with_single_result(self): importer_log, ImporterLogEntry.Category.COURSE, "Sheet "BA Belegungen", row 2 and 1 other place: " - + "Course "Shake" already exists in this semester, but the courses can not be merged for the following reasons:
" - + "- the evaluation of the existing course is a single result", + "Course "Shake" already exists in this semester, but the courses can not be merged for the following reasons:
" + "- the evaluation of the existing course is a single result", ) def test_existing_course_equal_except_evaluations(self): @@ -790,8 +790,8 @@ def test_existing_course_equal_except_evaluations(self): importer_log, ImporterLogEntry.Category.COURSE, "Sheet "BA Belegungen", row 2 and 1 other place: Course "Shake" already exists in " - + "this semester, but the courses can not be merged for the following reasons:" - + "
- the existing course does not have exactly one evaluation", + "this semester, but the courses can not be merged for the following reasons:" + "
- the existing course does not have exactly one evaluation", ) def test_existing_course_different_grading(self): @@ -809,8 +809,8 @@ def test_existing_course_different_grading(self): importer_log, ImporterLogEntry.Category.COURSE, "Sheet "BA Belegungen", row 2 and 1 other place: Course "Shake" already exists in this " - + "semester, but the courses can not be merged for the following reasons:" - + "
- the evaluation of the existing course has a mismatching grading specification", + "semester, but the courses can not be merged for the following reasons:" + "
- the evaluation of the existing course has a mismatching grading specification", ) def test_wrong_column_count(self): diff --git a/evap/staff/tests/test_views.py b/evap/staff/tests/test_views.py index ab6076122b..ead611aef1 100644 --- a/evap/staff/tests/test_views.py +++ b/evap/staff/tests/test_views.py @@ -1354,8 +1354,8 @@ def test_view_downloads_csv_file(self): ) expected_content = ( "Evaluation id;Course type;Course degrees;Vote end date;Timestamp\n" - + f"{self.evaluation_id};Type;;{self.vote_end_date};{self.timestamp_time}\n" - ).encode("utf-8") + f"{self.evaluation_id};Type;;{self.vote_end_date};{self.timestamp_time}\n" + ).encode() self.assertEqual(response.content, expected_content) diff --git a/pyproject.toml b/pyproject.toml index 1cabd4615b..a37dd7fb35 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4"] # COM,ISC,INP,PIE,PYI,Q,RSE,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC"] # COM,INP,PIE,PYI,Q,RSE,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible From 587e99edd6514e20fafd0d2a9557cbad9d63fb1d Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 21:31:49 +0100 Subject: [PATCH 18/54] enable and (auto) fix unnecessary start in range --- evap/evaluation/migrations/0100_clear_evaluation_names.py | 2 +- evap/evaluation/models.py | 2 +- evap/evaluation/tests/test_commands.py | 4 ++-- evap/results/exporters.py | 2 +- evap/staff/fixtures/__init__.py | 0 evap/staff/forms.py | 4 ++-- pyproject.toml | 5 +++-- 7 files changed, 10 insertions(+), 9 deletions(-) create mode 100644 evap/staff/fixtures/__init__.py diff --git a/evap/evaluation/migrations/0100_clear_evaluation_names.py b/evap/evaluation/migrations/0100_clear_evaluation_names.py index 11ef39ae82..50b96f0074 100644 --- a/evap/evaluation/migrations/0100_clear_evaluation_names.py +++ b/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})" diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index e3077fdf2f..5c585621cf 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -1905,7 +1905,7 @@ def ensure_valid_login_key(self): return while True: - key = secrets.choice(range(0, UserProfile.MAX_LOGIN_KEY)) + key = secrets.choice(range(UserProfile.MAX_LOGIN_KEY)) try: self.login_key = key self.reset_login_key_validity() diff --git a/evap/evaluation/tests/test_commands.py b/evap/evaluation/tests/test_commands.py index 1208b70054..ef31dbb5fb 100644 --- a/evap/evaluation/tests/test_commands.py +++ b/evap/evaluation/tests/test_commands.py @@ -336,7 +336,7 @@ def test_dont_remind_already_voted(self): self.assertEqual(mock.call_count, 0) self.assertEqual(len(mail.outbox), 0) - @override_settings(TEXTANSWER_REVIEW_REMINDER_WEEKDAYS=list(range(0, 8))) + @override_settings(TEXTANSWER_REVIEW_REMINDER_WEEKDAYS=list(range(8))) def test_send_text_answer_review_reminder(self): make_manager() evaluation = baker.make( @@ -409,7 +409,7 @@ def test_subcommands_called(self, mock_call_command, mock_subprocess_run): mock_call_command.assert_any_call("format") -@override_settings(TEXTANSWER_REVIEW_REMINDER_WEEKDAYS=range(0, 7)) +@override_settings(TEXTANSWER_REVIEW_REMINDER_WEEKDAYS=range(7)) class TestSendTextanswerRemindersCommand(TestCase): def test_send_reminder(self): make_manager() diff --git a/evap/results/exporters.py b/evap/results/exporters.py index 6c191af8ca..cc5c0cbafc 100644 --- a/evap/results/exporters.py +++ b/evap/results/exporters.py @@ -76,7 +76,7 @@ def init_grade_styles(cls): return grade_base_style = "pattern: pattern solid, fore_colour {}; alignment: horiz centre; font: bold on; borders: left medium, right medium" - for i in range(0, cls.NUM_GRADE_COLORS): + for i in range(cls.NUM_GRADE_COLORS): grade = 1 + i * cls.STEP color = get_grade_color(grade) palette_index = cls.CUSTOM_COLOR_START + i diff --git a/evap/staff/fixtures/__init__.py b/evap/staff/fixtures/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/evap/staff/forms.py b/evap/staff/forms.py index 0889de64a5..6bf659ab98 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -817,7 +817,7 @@ def handle_moved_contributors(data, **kwargs): evaluation = None total_forms = int(data["contributions-TOTAL_FORMS"]) - for i in range(0, total_forms): + for i in range(total_forms): prefix = "contributions-" + str(i) + "-" current_id = data.get(prefix + "id", "") contributor = data.get(prefix + "contributor", "") @@ -833,7 +833,7 @@ def handle_moved_contributors(data, **kwargs): continue # find the form with that previous contribution and then swap the contributions - for j in range(0, total_forms): + for j in range(total_forms): other_prefix = "contributions-" + str(j) + "-" other_id = data[other_prefix + "id"] if other_id == previous_id: diff --git a/pyproject.toml b/pyproject.toml index a37dd7fb35..5f9677935a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC"] # COM,INP,PIE,PYI,Q,RSE,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE"] # COM,PYI,Q,RSE,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible @@ -38,8 +38,9 @@ pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert metho [tool.ruff.per-file-ignores] "**/migrations/*.py" = ["N806"] # migrations have model classes as local variables, we use PascalCase for these - "**/commands/*.py" = ["T201"] # printing in commands is ok +"deployment/*.py" = ["INP001"] # not intendet for direct import + [tool.ruff.format] exclude = ["**/urls.py", "**/migrations/*.py"] From 61b5a789f16527693621ad94e44350dfc64a9ed0 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 21:38:49 +0100 Subject: [PATCH 19/54] enable and (auto) fix exception parentheses --- evap/evaluation/models.py | 8 ++++---- evap/rewards/tools.py | 8 ++++---- evap/staff/importers/enrollment.py | 4 ++-- pyproject.toml | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 5c585621cf..81ca9ac828 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -106,7 +106,7 @@ def results_can_be_archived(self): @transaction.atomic def archive(self): if not self.participations_can_be_archived: - raise NotArchivableError() + raise NotArchivableError for evaluation in self.evaluations.all(): evaluation._archive() self.participations_are_archived = True @@ -119,14 +119,14 @@ def delete_grade_documents(self): from evap.grades.models import GradeDocument if not self.grade_documents_can_be_deleted: - raise NotArchivableError() + raise NotArchivableError GradeDocument.objects.filter(course__semester=self).delete() self.grade_documents_are_deleted = True self.save() def archive_results(self): if not self.results_can_be_archived: - raise NotArchivableError() + raise NotArchivableError self.results_are_archived = True self.save() @@ -636,7 +636,7 @@ def num_participants(self): def _archive(self): """Should be called only via Semester.archive""" if not self.participations_can_be_archived: - raise NotArchivableError() + raise NotArchivableError if self._participant_count is not None: assert self._voter_count is not None assert ( diff --git a/evap/rewards/tools.py b/evap/rewards/tools.py index 142925b066..7e5bae5fba 100644 --- a/evap/rewards/tools.py +++ b/evap/rewards/tools.py @@ -31,22 +31,22 @@ def save_redemptions(request, redemptions: dict[int, int], previous_redeemed_poi # check consistent previous redeemed points # do not validate reward points, to allow receiving points after page load if previous_redeemed_points != redeemed_points_of_user(request.user): - raise OutdatedRedemptionDataError() + raise OutdatedRedemptionDataError total_points_available = reward_points_of_user(request.user) total_points_redeemed = sum(redemptions.values()) if total_points_redeemed <= 0: - raise NoPointsSelectedError() + raise NoPointsSelectedError if total_points_redeemed > total_points_available: - raise NotEnoughPointsError() + raise NotEnoughPointsError for event_id in redemptions: if redemptions[event_id] > 0: event = get_object_or_404(RewardPointRedemptionEvent, pk=event_id) if event.redeem_end_date < date.today(): - raise RedemptionEventExpiredError() + raise RedemptionEventExpiredError RewardPointRedemption.objects.create(user_profile=request.user, value=redemptions[event_id], event=event) diff --git a/evap/staff/importers/enrollment.py b/evap/staff/importers/enrollment.py index c6ad63184c..690d5f8812 100644 --- a/evap/staff/importers/enrollment.py +++ b/evap/staff/importers/enrollment.py @@ -370,10 +370,10 @@ def set_course_merge_target(self, course_data: CourseData) -> None: if course_with_same_name_en != course_with_same_name_de: if course_with_same_name_en is not None: - raise self.NameEnCollisionError() + raise self.NameEnCollisionError if course_with_same_name_de is not None: - raise self.NameDeCollisionError() + raise self.NameDeCollisionError assert course_with_same_name_en is not None assert course_with_same_name_de is not None diff --git a/pyproject.toml b/pyproject.toml index 5f9677935a..01ec24fbce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE"] # COM,PYI,Q,RSE,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE"] # COM,PYI,Q,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible From 8446461063cc579ef3f2e468b4593a84bb1d9c8c Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 21:40:39 +0100 Subject: [PATCH 20/54] enable and (auto) fix unnecessary assignment --- evap/evaluation/auth.py | 6 ++---- evap/evaluation/management/commands/send_reminders.py | 5 +---- evap/evaluation/models.py | 4 +--- evap/grades/tests.py | 3 +-- evap/staff/staff_mode.py | 3 +-- evap/staff/views.py | 4 +--- pyproject.toml | 2 +- 7 files changed, 8 insertions(+), 19 deletions(-) diff --git a/evap/evaluation/auth.py b/evap/evaluation/auth.py index e5565b81e5..b70efde1b3 100644 --- a/evap/evaluation/auth.py +++ b/evap/evaluation/auth.py @@ -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: diff --git a/evap/evaluation/management/commands/send_reminders.py b/evap/evaluation/management/commands/send_reminders.py index d3b47372c8..7b94a798c8 100644 --- a/evap/evaluation/management/commands/send_reminders.py +++ b/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 diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 81ca9ac828..3f774ffbea 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -2108,7 +2108,7 @@ def construct_mail(self, to_email, cc_addresses, subject_params, body_params): wrapper_template_params = {"email_content": rendered_content, "email_subject": subject, **body_params} wrapped_content = render_to_string("email_base.html", wrapper_template_params) - mail = EmailMultiAlternatives( + return EmailMultiAlternatives( subject=subject, body=plain_content, to=[to_email], @@ -2118,8 +2118,6 @@ def construct_mail(self, to_email, cc_addresses, subject_params, body_params): alternatives=[(wrapped_content, "text/html")], ) - return mail - @classmethod def send_reminder_to_user(cls, user, first_due_in_days, due_evaluations): template = cls.objects.get(name=cls.STUDENT_REMINDER) diff --git a/evap/grades/tests.py b/evap/grades/tests.py index 1e02bfc279..1e945a95e7 100644 --- a/evap/grades/tests.py +++ b/evap/grades/tests.py @@ -61,14 +61,13 @@ def helper_upload_grades(self, course, final_grades): upload_files = [("file", "grades.txt", b"Some content")] final = "?final=true" if final_grades else "" - response = self.app.post( + return self.app.post( f"{reverse('grades:upload_grades', args=[course.id])}{final}", params={"description_en": "Grades", "description_de": "Grades"}, user=self.grade_publisher, content_type="multipart/form-data", upload_files=upload_files, ).follow(status=200) - return response def helper_check_final_grade_upload(self, course, expected_number_of_emails): response = self.helper_upload_grades(course, final_grades=True) diff --git a/evap/staff/staff_mode.py b/evap/staff/staff_mode.py index ead425d0d3..0ba8300ba7 100644 --- a/evap/staff/staff_mode.py +++ b/evap/staff/staff_mode.py @@ -42,8 +42,7 @@ def middleware(request): request.user.is_manager = False request.user.is_reviewer = False - response = get_response(request) - return response + return get_response(request) return middleware diff --git a/evap/staff/views.py b/evap/staff/views.py index cf0d6346f8..326d66ed3a 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -175,9 +175,7 @@ def get_evaluations_with_prefetched_data(semester): ) ).order_by("pk") evaluations = annotate_evaluations_with_grade_document_counts(evaluations) - evaluations = Evaluation.annotate_with_participant_and_voter_counts(evaluations) - - return evaluations + return Evaluation.annotate_with_participant_and_voter_counts(evaluations) @reviewer_required diff --git a/pyproject.toml b/pyproject.toml index 01ec24fbce..42bd9ac9de 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE"] # COM,PYI,Q,RET,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET"] # COM,PYI,Q,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible From c0a850269ff216df455ed772455a57506049ae99 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 21:54:30 +0100 Subject: [PATCH 21/54] enable COMpatible COM --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 42bd9ac9de..3e8ce1d767 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,12 +25,13 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET"] # COM,PYI,Q,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM"] # PYI,Q,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible "A003", # we shadow some builtins sometime "DJ008", # do not use __str__ where it is not required + "COM812", # incompatible with formatter ] ignore-init-module-imports = true From 5830d4b07c260994c4f9af2792b0807de362b9a3 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 21:59:51 +0100 Subject: [PATCH 22/54] enable and fix unspecific ignore --- evap/evaluation/models.py | 22 +++++++++++----------- evap/evaluation/tools.py | 2 +- evap/results/views.py | 2 +- evap/settings.py | 2 +- evap/staff/forms.py | 4 ++-- evap/staff/importers/base.py | 2 +- evap/staff/importers/enrollment.py | 2 +- pyproject.toml | 2 +- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 3f774ffbea..6a367c8aa2 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -1291,7 +1291,7 @@ class BipolarChoices(NamedTuple): _("No answer"), ], is_inverted=False, - **BASE_UNIPOLAR_CHOICES, # type: ignore + **BASE_UNIPOLAR_CHOICES, # type: ignore[arg-type] ), QuestionType.NEGATIVE_LIKERT: Choices( names=[ @@ -1303,7 +1303,7 @@ class BipolarChoices(NamedTuple): _("No answer"), ], is_inverted=True, - **BASE_UNIPOLAR_CHOICES, # type: ignore + **BASE_UNIPOLAR_CHOICES, # type: ignore[arg-type] ), QuestionType.GRADE: Choices( names=[ @@ -1315,7 +1315,7 @@ class BipolarChoices(NamedTuple): _("No answer"), ], is_inverted=False, - **BASE_UNIPOLAR_CHOICES, # type: ignore + **BASE_UNIPOLAR_CHOICES, # type: ignore[arg-type] ), QuestionType.EASY_DIFFICULT: BipolarChoices( minus_name=_("Easy"), @@ -1330,7 +1330,7 @@ class BipolarChoices(NamedTuple): _("Way too\ndifficult"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, # type: ignore + **BASE_BIPOLAR_CHOICES, # type: ignore[arg-type] ), QuestionType.FEW_MANY: BipolarChoices( minus_name=_("Few"), @@ -1345,7 +1345,7 @@ class BipolarChoices(NamedTuple): _("Way too\nmany"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, # type: ignore + **BASE_BIPOLAR_CHOICES, # type: ignore[arg-type] ), QuestionType.LITTLE_MUCH: BipolarChoices( minus_name=_("Little"), @@ -1360,7 +1360,7 @@ class BipolarChoices(NamedTuple): _("Way too\nmuch"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, # type: ignore + **BASE_BIPOLAR_CHOICES, # type: ignore[arg-type] ), QuestionType.SMALL_LARGE: BipolarChoices( minus_name=_("Small"), @@ -1375,7 +1375,7 @@ class BipolarChoices(NamedTuple): _("Way too\nlarge"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, # type: ignore + **BASE_BIPOLAR_CHOICES, # type: ignore[arg-type] ), QuestionType.SLOW_FAST: BipolarChoices( minus_name=_("Slow"), @@ -1390,7 +1390,7 @@ class BipolarChoices(NamedTuple): _("Way too\nfast"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, # type: ignore + **BASE_BIPOLAR_CHOICES, # type: ignore[arg-type] ), QuestionType.SHORT_LONG: BipolarChoices( minus_name=_("Short"), @@ -1405,7 +1405,7 @@ class BipolarChoices(NamedTuple): _("Way too\nlong"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, # type: ignore + **BASE_BIPOLAR_CHOICES, # type: ignore[arg-type] ), QuestionType.POSITIVE_YES_NO: Choices( names=[ @@ -1414,7 +1414,7 @@ class BipolarChoices(NamedTuple): _("No answer"), ], is_inverted=False, - **BASE_YES_NO_CHOICES, # type: ignore + **BASE_YES_NO_CHOICES, # type: ignore[arg-type] ), QuestionType.NEGATIVE_YES_NO: Choices( names=[ @@ -1423,7 +1423,7 @@ class BipolarChoices(NamedTuple): _("No answer"), ], is_inverted=True, - **BASE_YES_NO_CHOICES, # type: ignore + **BASE_YES_NO_CHOICES, # type: ignore[arg-type] ), } diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index 07020c75cc..0d81abc8e8 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -228,7 +228,7 @@ def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) del self["content-type"] - @HttpResponse.content.setter # type: ignore + @HttpResponse.content.setter # type: ignore[attr-defined] def content(self, value): if value: raise AttributeError("You cannot set content to a 204 (No Content) response") diff --git a/evap/results/views.py b/evap/results/views.py index 92caa09de0..8f599e37aa 100644 --- a/evap/results/views.py +++ b/evap/results/views.py @@ -105,7 +105,7 @@ def update_template_cache_of_published_evaluations_in_course(course): def get_evaluations_with_prefetched_data(evaluations): - if isinstance(evaluations, QuerySet): # type: ignore + if isinstance(evaluations, QuerySet): # type: ignore[misc] evaluations = evaluations.select_related("course__type").prefetch_related( "course__degrees", "course__semester", diff --git a/evap/settings.py b/evap/settings.py index 30be729665..b06b79a4ac 100644 --- a/evap/settings.py +++ b/evap/settings.py @@ -410,7 +410,7 @@ def CHARACTER_ALLOWED_IN_NAME(character): # noqa: N802 # localsettings file may exist (vagrant) or do not (GitHub) # the import can overwrite locals with a slightly different type (e.g. DATABASES), which is fine. - from evap.localsettings import * # type: ignore # noqa: F403 + from evap.localsettings import * # type: ignore # noqa: F403,PGH003 except ImportError: pass diff --git a/evap/staff/forms.py b/evap/staff/forms.py index 6bf659ab98..1a067e5ff4 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -260,7 +260,7 @@ def validate_unique(self): self.add_error(name_field, e) -class CourseForm(CourseFormMixin, forms.ModelForm): # type: ignore +class CourseForm(CourseFormMixin, forms.ModelForm): # type: ignore[misc] semester = forms.ModelChoiceField(Semester.objects.all(), disabled=True, required=False, widget=forms.HiddenInput()) def __init__(self, *args, **kwargs): @@ -270,7 +270,7 @@ def __init__(self, *args, **kwargs): disable_all_fields(self) -class CourseCopyForm(CourseFormMixin, forms.ModelForm): # type: ignore +class CourseCopyForm(CourseFormMixin, forms.ModelForm): # type: ignore[misc] semester = forms.ModelChoiceField(Semester.objects.all()) vote_start_datetime = forms.DateTimeField(label=_("Start of evaluations"), localize=True) vote_end_date = forms.DateField(label=_("Last day of evaluations"), localize=True) diff --git a/evap/staff/importers/base.py b/evap/staff/importers/base.py index 7a9ba97f3a..a2ce20ec08 100644 --- a/evap/staff/importers/base.py +++ b/evap/staff/importers/base.py @@ -208,7 +208,7 @@ def map(self, file_content: bytes): ) from e rows = [] - for sheet in book: # type: ignore + for sheet in book: # type: ignore[attr-defined] if sheet.max_row <= self.skip_first_n_rows: continue diff --git a/evap/staff/importers/enrollment.py b/evap/staff/importers/enrollment.py index 690d5f8812..03f1413a3f 100644 --- a/evap/staff/importers/enrollment.py +++ b/evap/staff/importers/enrollment.py @@ -552,7 +552,7 @@ def finalize(self) -> None: seen_evaluation_names = self.participant_emails_per_course_name_en.keys() existing_participation_pairs = [ - (participation.evaluation.course.name_en, participation.userprofile.email) # type: ignore + (participation.evaluation.course.name_en, participation.userprofile.email) # type: ignore[misc] for participation in Evaluation.participants.through._default_manager.filter( evaluation__course__name_en__in=seen_evaluation_names, userprofile__email__in=seen_user_emails ).prefetch_related("userprofile", "evaluation__course") diff --git a/pyproject.toml b/pyproject.toml index 3e8ce1d767..36db3e4d2f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM"] # PYI,Q,SIM,PTH,PGH,PL,TRY,FLY,PERF,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH"] # PYI,Q,SIM,PTH,PL,TRY,FLY,PERF,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible From 333889ed428d7bde374606fa24ab58ebe91e9a6e Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 22:09:23 +0100 Subject: [PATCH 23/54] enable and fix string join --- evap/staff/tools.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/evap/staff/tools.py b/evap/staff/tools.py index 07cebcf411..98bf9fe413 100644 --- a/evap/staff/tools.py +++ b/evap/staff/tools.py @@ -228,7 +228,7 @@ def merge_users(main_user, other_user, preview=False): merged_user["first_name_given"] = main_user.first_name_given or other_user.first_name_given or "" merged_user["last_name"] = main_user.last_name or other_user.last_name or "" merged_user["email"] = main_user.email or other_user.email or None - merged_user["notes"] = "\n".join((main_user.notes, other_user.notes)).strip() + merged_user["notes"] = f"{main_user.notes}\n{other_user.notes}".strip() merged_user["groups"] = Group.objects.filter(user__in=[main_user, other_user]).distinct() merged_user["is_superuser"] = main_user.is_superuser or other_user.is_superuser diff --git a/pyproject.toml b/pyproject.toml index 36db3e4d2f..7a3d6ff518 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH"] # PYI,Q,SIM,PTH,PL,TRY,FLY,PERF,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY"] # PYI,Q,SIM,PTH,PL,TRY,FLY,PERF,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible From 22d4a7961201c56b90bbbb57d79e5d1eca1f7dd1 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 13 Nov 2023 22:10:55 +0100 Subject: [PATCH 24/54] enable and fix list comprehensions --- .../management/commands/send_reminders.py | 9 ++++---- evap/rewards/views.py | 21 +++++++++++-------- evap/staff/tools.py | 5 ++--- evap/staff/views.py | 6 +++--- pyproject.toml | 3 ++- 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/evap/evaluation/management/commands/send_reminders.py b/evap/evaluation/management/commands/send_reminders.py index 7b94a798c8..2912215f7d 100644 --- a/evap/evaluation/management/commands/send_reminders.py +++ b/evap/evaluation/management/commands/send_reminders.py @@ -40,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( diff --git a/evap/rewards/views.py b/evap/rewards/views.py index 42568de6aa..7a727f546c 100644 --- a/evap/rewards/views.py +++ b/evap/rewards/views.py @@ -78,15 +78,18 @@ def index(request): reward_point_redemptions = RewardPointRedemption.objects.filter(user_profile=request.user) events = RewardPointRedemptionEvent.objects.filter(redeem_end_date__gte=datetime.now()).order_by("date") - reward_point_actions = [] - for granting in reward_point_grantings: - reward_point_actions.append( - (granting.granting_time, _("Reward for") + " " + granting.semester.name, granting.value, "") - ) - for redemption in reward_point_redemptions: - reward_point_actions.append((redemption.redemption_time, redemption.event.name, "", redemption.value)) - - reward_point_actions.sort(key=lambda action: action[0], reverse=True) + granted_point_actions = [ + (granting.granting_time, _("Reward for") + " " + granting.semester.name, granting.value, "") + for granting in reward_point_grantings + ] + redemption_point_actions = [ + (redemption.redemption_time, redemption.event.name, "", redemption.value) + for redemption in reward_point_redemptions + ] + + reward_point_actions = sorted( + granted_point_actions + redemption_point_actions, key=lambda action: action[0], reverse=True + ) template_data = { "reward_point_actions": reward_point_actions, diff --git a/evap/staff/tools.py b/evap/staff/tools.py index 98bf9fe413..d38e382ab8 100644 --- a/evap/staff/tools.py +++ b/evap/staff/tools.py @@ -206,9 +206,8 @@ def bulk_update_users(request, user_file_content, test_run): for user, email in users_to_be_updated: user.email = email user.save() - userprofiles_to_create = [] - for email in emails_of_users_to_be_created: - userprofiles_to_create.append(UserProfile(email=email)) + userprofiles_to_create = [UserProfile(email=email) for email in emails_of_users_to_be_created] + UserProfile.objects.bulk_create(userprofiles_to_create) messages.success(request, _("Users have been successfully updated.")) diff --git a/evap/staff/views.py b/evap/staff/views.py index 326d66ed3a..bac24c60bc 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -691,9 +691,9 @@ def semester_export(request, semester_id): if formset.is_valid(): include_not_enough_voters = request.POST.get("include_not_enough_voters") == "on" include_unpublished = request.POST.get("include_unpublished") == "on" - selection_list = [] - for form in formset: - selection_list.append((form.cleaned_data["selected_degrees"], form.cleaned_data["selected_course_types"])) + selection_list = [ + (form.cleaned_data["selected_degrees"], form.cleaned_data["selected_course_types"]) for form in formset + ] filename = f"Evaluation-{semester.name}-{get_language()}.xls" response = AttachmentResponse(filename, content_type="application/vnd.ms-excel") diff --git a/pyproject.toml b/pyproject.toml index 7a3d6ff518..764f642d5f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,13 +25,14 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY"] # PYI,Q,SIM,PTH,PL,TRY,FLY,PERF,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF"] # PYI,Q,SIM,PTH,PL,TRY,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible "A003", # we shadow some builtins sometime "DJ008", # do not use __str__ where it is not required "COM812", # incompatible with formatter + "PERF203", ] ignore-init-module-imports = true From 3173077a1c58a00d0073cf03205614c228a12536 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 20 Nov 2023 17:41:11 +0100 Subject: [PATCH 25/54] enable some pylint rules --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 764f642d5f..399f4f9ba5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF"] # PYI,Q,SIM,PTH,PL,TRY,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF", "PLE"] # PYI,Q,SIM,PTH,PL,TRY,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible From 07770caf3a8e8a4d99d00190692ce65c28aa81ad Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 20 Nov 2023 18:25:46 +0100 Subject: [PATCH 26/54] rerun ci From 002736a7fff5650bb56da8c953695dab1e1e9de3 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 20 Nov 2023 19:01:46 +0100 Subject: [PATCH 27/54] attr defined mypy ignore --- evap/evaluation/tools.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index 0d81abc8e8..a6640e0934 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -67,15 +67,15 @@ def discard_cached_related_objects(instance: M) -> M: hierarchy (e.g. for storing instances in a cache) """ # Extracted from django's refresh_from_db, which sadly doesn't offer this part alone (without hitting the DB). - for field in instance._meta.concrete_fields: # type: ignore + for field in instance._meta.concrete_fields: # type: ignore[attr-defined] if field.is_relation and field.is_cached(instance): field.delete_cached_value(instance) - for field in instance._meta.related_objects: # type: ignore + for field in instance._meta.related_objects: # type: ignore[attr-defined] if field.is_cached(instance): field.delete_cached_value(instance) - instance._prefetched_objects_cache = {} # type: ignore + instance._prefetched_objects_cache = {} # type: ignore[attr-defined] return instance From 6aecca3f9de01fb52f3e29571bea2371b6f4dc9c Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Sun, 24 Dec 2023 09:53:09 +0100 Subject: [PATCH 28/54] Fix spelling and grammar Co-authored-by: Richard Ebeling --- evap/settings.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/evap/settings.py b/evap/settings.py index b06b79a4ac..3177ef352f 100644 --- a/evap/settings.py +++ b/evap/settings.py @@ -407,7 +407,7 @@ def CHARACTER_ALLOWED_IN_NAME(character): # noqa: N802 # Create a localsettings.py if you want to locally override settings # and don't want the changes to appear in 'git status'. try: - # localsettings file may exist (vagrant) or do not (GitHub) + # localsettings file may (vagrant) or may not (CI run) exist # the import can overwrite locals with a slightly different type (e.g. DATABASES), which is fine. from evap.localsettings import * # type: ignore # noqa: F403,PGH003 diff --git a/pyproject.toml b/pyproject.toml index 399f4f9ba5..ed632dc078 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,7 +41,7 @@ pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert metho [tool.ruff.per-file-ignores] "**/migrations/*.py" = ["N806"] # migrations have model classes as local variables, we use PascalCase for these "**/commands/*.py" = ["T201"] # printing in commands is ok -"deployment/*.py" = ["INP001"] # not intendet for direct import +"deployment/*.py" = ["INP001"] # not intended for direct import [tool.ruff.format] exclude = ["**/urls.py", "**/migrations/*.py"] From afaf0989f4a0e42b7d2994fbb5a43c167490a1b6 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Fri, 29 Dec 2023 12:49:57 +0100 Subject: [PATCH 29/54] enable PL, but keep invalid-name --- evap/evaluation/models.py | 2 +- evap/evaluation/views.py | 3 +-- evap/results/tools.py | 3 +-- evap/settings.py | 2 +- evap/staff/tools.py | 8 ++++---- pyproject.toml | 9 +++++---- 6 files changed, 13 insertions(+), 14 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 6a367c8aa2..05eb5308ff 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -444,7 +444,7 @@ class State: ) class TextAnswerReviewState(Enum): - do_not_call_in_templates = True + do_not_call_in_templates = True # pylint: disable=invalid-name NO_TEXTANSWERS = auto() NO_REVIEW_NEEDED = auto() REVIEW_NEEDED = auto() diff --git a/evap/evaluation/views.py b/evap/evaluation/views.py index a21616c1c9..77d6cd48da 100644 --- a/evap/evaluation/views.py +++ b/evap/evaluation/views.py @@ -23,8 +23,7 @@ logger = logging.getLogger(__name__) -def redirect_user_to_start_page(user): - # pylint: disable=too-many-return-statements +def redirect_user_to_start_page(user): # noqa: PLR0911 active_semester = Semester.active_semester() if user.is_reviewer: diff --git a/evap/results/tools.py b/evap/results/tools.py index 59480117d6..ca4485c223 100644 --- a/evap/results/tools.py +++ b/evap/results/tools.py @@ -458,13 +458,12 @@ def textanswers_visible_to(contribution): return TextAnswerVisibility(visible_by_contribution=sorted_contributors, visible_by_delegation_count=num_delegates) -def can_textanswer_be_seen_by( +def can_textanswer_be_seen_by( # noqa: PLR0911 user: UserProfile, represented_users: list[UserProfile], textanswer: TextAnswer, view: str, ) -> bool: - # pylint: disable=too-many-return-statements assert textanswer.review_decision in [TextAnswer.ReviewDecision.PRIVATE, TextAnswer.ReviewDecision.PUBLIC] contributor = textanswer.contribution.contributor diff --git a/evap/settings.py b/evap/settings.py index 3177ef352f..e895a84840 100644 --- a/evap/settings.py +++ b/evap/settings.py @@ -375,7 +375,7 @@ class ManifestStaticFilesStorageWithJsReplacement(ManifestStaticFilesStorage): ### Allowed chosen first names / display names -def CHARACTER_ALLOWED_IN_NAME(character): # noqa: N802 +def CHARACTER_ALLOWED_IN_NAME(character): # pylint: disable=invalid-name return any( ( ord(character) in range(32, 127), # printable ASCII / Basic Latin characters diff --git a/evap/staff/tools.py b/evap/staff/tools.py index d38e382ab8..ac289daa54 100644 --- a/evap/staff/tools.py +++ b/evap/staff/tools.py @@ -100,7 +100,7 @@ def find_matching_internal_user_for_email(request, email): return matching_users[0] -def bulk_update_users(request, user_file_content, test_run): +def bulk_update_users(request, user_file_content, test_run): # noqa: PLR0912 # pylint: disable=too-many-branches,too-many-locals # user_file must have one user per line in the format "{username},{email}" imported_emails = {clean_email(line.decode().split(",")[1]) for line in user_file_content.splitlines()} @@ -215,10 +215,10 @@ def bulk_update_users(request, user_file_content, test_run): @transaction.atomic -def merge_users(main_user, other_user, preview=False): +def merge_users( # noqa: PLR0915 # This is much stuff to do. However, splitting it up into subtasks doesn't make much sense. + main_user, other_user, preview=False +): """Merges other_user into main_user""" - # This is much stuff to do. However, splitting it up into subtasks doesn't make much sense. - # pylint: disable=too-many-statements merged_user = {} merged_user["is_active"] = main_user.is_active or other_user.is_active diff --git a/pyproject.toml b/pyproject.toml index ed632dc078..499dda0f0b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF", "PLE"] # PYI,Q,SIM,PTH,PL,TRY,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF", "PL"] # PYI,Q,SIM,PTH,TRY,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible @@ -33,6 +33,9 @@ ignore = [ "DJ008", # do not use __str__ where it is not required "COM812", # incompatible with formatter "PERF203", + "N802", # not as mighty as pylint's invalid-name https://github.com/astral-sh/ruff/issues/7660 + "PLR0913", # we can't determine a good limit for arguements. reviews should spot bad cases of this. + "PLR2004", "PLW2901", # not activated pylint extensions ] ignore-init-module-imports = true @@ -77,7 +80,6 @@ enable = ["all"] disable = [ "locally-disabled", # we allow locally disabling some checks if we think it makes sense to do that. "suppressed-message", - "line-too-long", # black does code formatting for us "ungrouped-imports", # isort "wrong-import-order", # isort "too-many-public-methods", # reported for some models, that won't change @@ -88,13 +90,12 @@ disable = [ "missing-docstring", # yeah... we don't have those "protected-access", # for us that means "be careful", not "don't do it" "too-many-lines", # we don't currently think that splitting up views.py or test_views.py creates any value - "too-many-arguments", # we can't determine a good limit here. reviews should spot bad cases of this. "duplicate-code", # Mostly imports and test setup. "cyclic-import", # We use these inside methods that require models from multiple apps. Tests will catch actual errors. "unsupported-binary-operation", # broken in pylint: https://github.com/PyCQA/pylint/issues/7381 "use-implicit-booleaness-not-comparison-to-string", # forces us to use less expressive code "use-implicit-booleaness-not-comparison-to-zero", # forces us to use less expressive code - "unused-wildcard-import", "wildcard-import", "invalid-name", # covered by ruff + "line-too-long","unused-wildcard-import", "wildcard-import","too-many-arguments", "too-many-statements", "too-many-return-statements", # covered by ruff ] ############################################## From 3c06cf808abced5b9c866dcd80476d33564adab5 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Fri, 29 Dec 2023 14:18:12 +0100 Subject: [PATCH 30/54] enable PERF203 and BLE instead of broad-exception-caught --- evap/evaluation/models.py | 4 ++-- evap/staff/importers/base.py | 2 +- evap/staff/views.py | 2 +- pyproject.toml | 5 ++--- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 05eb5308ff..1c48558206 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -950,7 +950,7 @@ def update_evaluations(cls): evaluation.publish() evaluation_results_evaluations.append(evaluation) evaluation.save() - except Exception: # pylint: disable=broad-except + except Exception: # noqa: PERF203 logger.exception( 'An error occured when updating the state of evaluation "%s" (id %d).', evaluation, evaluation.id ) @@ -2092,7 +2092,7 @@ def send_to_user(self, user, subject_params, body_params, use_cc, additional_cc_ logger.info('Sent email "%s" to %s.', mail.subject, user.full_name_with_additional_info) if send_separate_login_url: self.send_login_url_to_user(user) - except Exception: # pylint: disable=broad-except + except Exception: logger.exception( 'An exception occurred when sending the following email to user "%s":\n%s\n', user.full_name_with_additional_info, diff --git a/evap/staff/importers/base.py b/evap/staff/importers/base.py index a2ce20ec08..d4b7668402 100644 --- a/evap/staff/importers/base.py +++ b/evap/staff/importers/base.py @@ -201,7 +201,7 @@ def __init__(self, skip_first_n_rows: int, row_cls: type[InputRow], importer_log def map(self, file_content: bytes): try: book = openpyxl.load_workbook(BytesIO(file_content)) - except Exception as e: + except Exception as e: # noqa: BLE001 raise ImporterError( message=_("Couldn't read the file. Error: {}").format(e), category=ImporterLogEntry.Category.SCHEMA, diff --git a/evap/staff/views.py b/evap/staff/views.py index bac24c60bc..27ccb3bedb 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -2267,7 +2267,7 @@ def user_bulk_update(request): success = False try: success = bulk_update_users(request, file_content, test_run) - except Exception: # pylint: disable=broad-except + except Exception: # noqa: BLE001 if settings.DEBUG: raise messages.error( diff --git a/pyproject.toml b/pyproject.toml index 499dda0f0b..c54529fc6c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,14 +25,13 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF", "PL"] # PYI,Q,SIM,PTH,TRY,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF", "PL", "BLE"] # PYI,Q,SIM,PTH,TRY,RUF ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible "A003", # we shadow some builtins sometime "DJ008", # do not use __str__ where it is not required "COM812", # incompatible with formatter - "PERF203", "N802", # not as mighty as pylint's invalid-name https://github.com/astral-sh/ruff/issues/7660 "PLR0913", # we can't determine a good limit for arguements. reviews should spot bad cases of this. "PLR2004", "PLW2901", # not activated pylint extensions @@ -95,7 +94,7 @@ disable = [ "unsupported-binary-operation", # broken in pylint: https://github.com/PyCQA/pylint/issues/7381 "use-implicit-booleaness-not-comparison-to-string", # forces us to use less expressive code "use-implicit-booleaness-not-comparison-to-zero", # forces us to use less expressive code - "line-too-long","unused-wildcard-import", "wildcard-import","too-many-arguments", "too-many-statements", "too-many-return-statements", # covered by ruff + "broad-exception-caught", "line-too-long","unused-wildcard-import", "wildcard-import","too-many-arguments", "too-many-statements", "too-many-return-statements", # covered by ruff ] ############################################## From 9e88552f95832945ad8c4bcc859f33af8ac342dd Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Fri, 29 Dec 2023 14:27:14 +0100 Subject: [PATCH 31/54] add pylint disable (why?!) --- evap/staff/fixtures/excel_files_test_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/staff/fixtures/excel_files_test_data.py b/evap/staff/fixtures/excel_files_test_data.py index 64fbd90fc1..e72f6825d1 100644 --- a/evap/staff/fixtures/excel_files_test_data.py +++ b/evap/staff/fixtures/excel_files_test_data.py @@ -198,7 +198,7 @@ ] } -random_file_content = b"Hallo Welt\n" +random_file_content = b"Hallo Welt\n" # pylint: disable=invalid-name wrong_column_count_excel_data = { From b1b969e227d4d1f149c145371609d6773b76ae08 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Fri, 29 Dec 2023 15:10:23 +0100 Subject: [PATCH 32/54] change warning stacklevel to 3 --- evap/results/exporters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/results/exporters.py b/evap/results/exporters.py index cc5c0cbafc..63f4f5a7cd 100644 --- a/evap/results/exporters.py +++ b/evap/results/exporters.py @@ -71,7 +71,7 @@ def init_grade_styles(cls): "This can happen, if the file is imported / run multiple " "times in one application run.", ImportWarning, - stacklevel=2, + stacklevel=3, ) return From d73931c65269f8773771fef872c957062241dfd3 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Jan 2024 19:27:16 +0100 Subject: [PATCH 33/54] don't break github --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c54529fc6c..fb3e6c3d76 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,7 +34,7 @@ ignore = [ "COM812", # incompatible with formatter "N802", # not as mighty as pylint's invalid-name https://github.com/astral-sh/ruff/issues/7660 "PLR0913", # we can't determine a good limit for arguements. reviews should spot bad cases of this. - "PLR2004", "PLW2901", # not activated pylint extensions + "PLR2004", "PLW2901" ] ignore-init-module-imports = true @@ -94,7 +94,7 @@ disable = [ "unsupported-binary-operation", # broken in pylint: https://github.com/PyCQA/pylint/issues/7381 "use-implicit-booleaness-not-comparison-to-string", # forces us to use less expressive code "use-implicit-booleaness-not-comparison-to-zero", # forces us to use less expressive code - "broad-exception-caught", "line-too-long","unused-wildcard-import", "wildcard-import","too-many-arguments", "too-many-statements", "too-many-return-statements", # covered by ruff + "broad-exception-caught", "line-too-long","unused-wildcard-import", "wildcard-import","too-many-arguments", "too-many-statements", "too-many-return-statements" ] ############################################## From b6bf08b96e79a3ec577787d274d1589d678c6472 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Jan 2024 19:48:09 +0100 Subject: [PATCH 34/54] don't break github 2 --- pyproject.toml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index fb3e6c3d76..babca6f8dc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,9 +41,12 @@ ignore-init-module-imports = true pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert methods use camelCase; Formsets use PascalCase [tool.ruff.per-file-ignores] -"**/migrations/*.py" = ["N806"] # migrations have model classes as local variables, we use PascalCase for these -"**/commands/*.py" = ["T201"] # printing in commands is ok -"deployment/*.py" = ["INP001"] # not intended for direct import +# migrations have model classes as local variables, we use PascalCase for these +"**/migrations/*.py" = ["N806"] +# printing in commands is ok +"**/commands/*.py" = ["T201"] +# not intended for direct import +"deployment/*.py" = ["INP001"] [tool.ruff.format] exclude = ["**/urls.py", "**/migrations/*.py"] From feda4b1e00b0b064d59acca2535bc0294dfdec88 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Jan 2024 19:49:22 +0100 Subject: [PATCH 35/54] don't break github 3 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index babca6f8dc..1a3247fd52 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,7 +41,7 @@ ignore-init-module-imports = true pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert methods use camelCase; Formsets use PascalCase [tool.ruff.per-file-ignores] -# migrations have model classes as local variables, we use PascalCase for these +# migrations have model classes as local variables; we use PascalCase for these "**/migrations/*.py" = ["N806"] # printing in commands is ok "**/commands/*.py" = ["T201"] From 6e978e12f3d6d62288b7f2a6d2cf2c5fcf018709 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Jan 2024 19:51:59 +0100 Subject: [PATCH 36/54] don't break github 4 --- pyproject.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1a3247fd52..85d4980498 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,11 +41,8 @@ ignore-init-module-imports = true pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert methods use camelCase; Formsets use PascalCase [tool.ruff.per-file-ignores] -# migrations have model classes as local variables; we use PascalCase for these "**/migrations/*.py" = ["N806"] -# printing in commands is ok "**/commands/*.py" = ["T201"] -# not intended for direct import "deployment/*.py" = ["INP001"] [tool.ruff.format] From b819c0e9f7cc4cff2d5120a92ff76b64e3280cfe Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Jan 2024 20:16:50 +0100 Subject: [PATCH 37/54] Revert "don't break github 4" This reverts commit 6e978e12f3d6d62288b7f2a6d2cf2c5fcf018709. --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 85d4980498..1a3247fd52 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,8 +41,11 @@ ignore-init-module-imports = true pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert methods use camelCase; Formsets use PascalCase [tool.ruff.per-file-ignores] +# migrations have model classes as local variables; we use PascalCase for these "**/migrations/*.py" = ["N806"] +# printing in commands is ok "**/commands/*.py" = ["T201"] +# not intended for direct import "deployment/*.py" = ["INP001"] [tool.ruff.format] From 659607216c32c9996e61546f600ae2524e946a80 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Jan 2024 20:17:04 +0100 Subject: [PATCH 38/54] Revert "don't break github 3" This reverts commit feda4b1e00b0b064d59acca2535bc0294dfdec88. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 1a3247fd52..babca6f8dc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,7 +41,7 @@ ignore-init-module-imports = true pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert methods use camelCase; Formsets use PascalCase [tool.ruff.per-file-ignores] -# migrations have model classes as local variables; we use PascalCase for these +# migrations have model classes as local variables, we use PascalCase for these "**/migrations/*.py" = ["N806"] # printing in commands is ok "**/commands/*.py" = ["T201"] From 1d8763c29aa5068fc5ff24db22b2a5de66366d09 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Jan 2024 20:18:15 +0100 Subject: [PATCH 39/54] Revert "don't break github 2" This reverts commit b6bf08b96e79a3ec577787d274d1589d678c6472. --- pyproject.toml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index babca6f8dc..fb3e6c3d76 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,12 +41,9 @@ ignore-init-module-imports = true pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert methods use camelCase; Formsets use PascalCase [tool.ruff.per-file-ignores] -# migrations have model classes as local variables, we use PascalCase for these -"**/migrations/*.py" = ["N806"] -# printing in commands is ok -"**/commands/*.py" = ["T201"] -# not intended for direct import -"deployment/*.py" = ["INP001"] +"**/migrations/*.py" = ["N806"] # migrations have model classes as local variables, we use PascalCase for these +"**/commands/*.py" = ["T201"] # printing in commands is ok +"deployment/*.py" = ["INP001"] # not intended for direct import [tool.ruff.format] exclude = ["**/urls.py", "**/migrations/*.py"] From 84a071274cc552251ab32caa5d50b9bcde73299d Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Jan 2024 21:00:20 +0100 Subject: [PATCH 40/54] bump ruff --- evap/evaluation/models.py | 3 --- requirements-dev.txt | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 1c48558206..ca9a346ab1 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -1529,9 +1529,6 @@ def is_private(self): def is_reviewed(self): return self.review_decision != self.ReviewDecision.UNDECIDED - def save(self, *args, **kwargs): - super().save(*args, **kwargs) - class FaqSection(models.Model): """Section in the frequently asked questions""" diff --git a/requirements-dev.txt b/requirements-dev.txt index 0eb95714e9..31aaff8be6 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -10,7 +10,7 @@ mypy~=1.8.0 openpyxl-stubs~=0.1.25 pylint-django~=2.5.4 pylint~=3.0.1 -ruff==0.1.5 +ruff==0.1.14 tblib~=3.0.0 xlrd~=2.0.1 typeguard~=4.1.5 From 22507ee2ea22a7e2cca62d76e3360c405f5ff5bf Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 29 Jan 2024 21:08:39 +0100 Subject: [PATCH 41/54] fix strict last unstrict zip (oops) --- evap/results/tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/results/tools.py b/evap/results/tools.py index bae14cf7f2..3c37007aad 100644 --- a/evap/results/tools.py +++ b/evap/results/tools.py @@ -103,7 +103,7 @@ def approval_count(self) -> int: class AnsweredRatingResult(PublishedRatingResult): @property def average(self) -> float: - return sum(grade * count for count, grade in zip(self.counts, self.choices.grades)) / self.count_sum + return sum(grade * count for count, grade in zip(self.counts, self.choices.grades, strict=True)) / self.count_sum class TextResult: From c787d4bb9ebd5eb2a6d7bd11c231debfb52084d2 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 12 Feb 2024 22:25:43 +0100 Subject: [PATCH 42/54] add comment and don't break github? --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index fb3e6c3d76..51cd8f2173 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,7 +94,7 @@ disable = [ "unsupported-binary-operation", # broken in pylint: https://github.com/PyCQA/pylint/issues/7381 "use-implicit-booleaness-not-comparison-to-string", # forces us to use less expressive code "use-implicit-booleaness-not-comparison-to-zero", # forces us to use less expressive code - "broad-exception-caught", "line-too-long","unused-wildcard-import", "wildcard-import","too-many-arguments", "too-many-statements", "too-many-return-statements" + "broad-exception-caught", "line-too-long","unused-wildcard-import", "wildcard-import","too-many-arguments", "too-many-statements", "too-many-return-statements" # covered by ruff ] ############################################## From 803753289032da654d33b1675780894a5a0ff561 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 12 Feb 2024 22:27:24 +0100 Subject: [PATCH 43/54] add comment and don't break github? again! --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 51cd8f2173..074a2a26d8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,7 +94,8 @@ disable = [ "unsupported-binary-operation", # broken in pylint: https://github.com/PyCQA/pylint/issues/7381 "use-implicit-booleaness-not-comparison-to-string", # forces us to use less expressive code "use-implicit-booleaness-not-comparison-to-zero", # forces us to use less expressive code - "broad-exception-caught", "line-too-long","unused-wildcard-import", "wildcard-import","too-many-arguments", "too-many-statements", "too-many-return-statements" # covered by ruff + # the following are covered by ruff + "broad-exception-caught", "line-too-long","unused-wildcard-import", "wildcard-import","too-many-arguments", "too-many-statements", "too-many-return-statements" ] ############################################## From b292520fb9e6b21ba28fe2b6f4f7381a258f8793 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 26 Feb 2024 17:41:32 +0100 Subject: [PATCH 44/54] Update pyproject.toml Co-authored-by: Richard Ebeling --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 074a2a26d8..8b22a5eab9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,7 +33,7 @@ ignore = [ "DJ008", # do not use __str__ where it is not required "COM812", # incompatible with formatter "N802", # not as mighty as pylint's invalid-name https://github.com/astral-sh/ruff/issues/7660 - "PLR0913", # we can't determine a good limit for arguements. reviews should spot bad cases of this. + "PLR0913", # we can't determine a good limit for arguments. reviews should spot bad cases of this. "PLR2004", "PLW2901" ] From 49290e4a09ab436cb1cc205e944fe88497d2c3f6 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 26 Feb 2024 17:41:59 +0100 Subject: [PATCH 45/54] Use correct weekday range --- evap/evaluation/tests/test_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/evaluation/tests/test_commands.py b/evap/evaluation/tests/test_commands.py index ef31dbb5fb..b419a208d2 100644 --- a/evap/evaluation/tests/test_commands.py +++ b/evap/evaluation/tests/test_commands.py @@ -336,7 +336,7 @@ def test_dont_remind_already_voted(self): self.assertEqual(mock.call_count, 0) self.assertEqual(len(mail.outbox), 0) - @override_settings(TEXTANSWER_REVIEW_REMINDER_WEEKDAYS=list(range(8))) + @override_settings(TEXTANSWER_REVIEW_REMINDER_WEEKDAYS=list(range(7))) def test_send_text_answer_review_reminder(self): make_manager() evaluation = baker.make( From f9f74c88684264353c8147500af93e2480df39d4 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 26 Feb 2024 17:59:46 +0100 Subject: [PATCH 46/54] change unaddressed rule comment --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 8b22a5eab9..fb740cf494 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,8 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 -select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF", "PL", "BLE"] # PYI,Q,SIM,PTH,TRY,RUF +# Those are interesting, but not yet addressed: PYI,Q,SIM,PTH,TRY,RUF +select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF", "PL", "BLE"] ignore = [ "E501", # line-too-long: black does code formatting for us "FIX004", # hacks should be possible From ea14cce484bbb3d0fd0f7044e699459d3028ac10 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 26 Feb 2024 18:03:29 +0100 Subject: [PATCH 47/54] fix errors introduced in latest changes --- evap/results/templatetags/results_templatetags.py | 3 ++- evap/results/tools.py | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/evap/results/templatetags/results_templatetags.py b/evap/results/templatetags/results_templatetags.py index 23f35a4a4c..38d9f41162 100644 --- a/evap/results/templatetags/results_templatetags.py +++ b/evap/results/templatetags/results_templatetags.py @@ -1,4 +1,5 @@ -from typing import Any, Iterable +from collections.abc import Iterable +from typing import Any from django.template import Library diff --git a/evap/results/tools.py b/evap/results/tools.py index 3c37007aad..0a9f045041 100644 --- a/evap/results/tools.py +++ b/evap/results/tools.py @@ -103,7 +103,9 @@ def approval_count(self) -> int: class AnsweredRatingResult(PublishedRatingResult): @property def average(self) -> float: - return sum(grade * count for count, grade in zip(self.counts, self.choices.grades, strict=True)) / self.count_sum + return ( + sum(grade * count for count, grade in zip(self.counts, self.choices.grades, strict=True)) / self.count_sum + ) class TextResult: From 5baa043e0539abd377ff44afd63f4fc6ddec0f50 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 26 Feb 2024 18:30:37 +0100 Subject: [PATCH 48/54] run ruff first and then pylint. Also do this with manage.py --- .github/workflows/tests.yml | 6 +++--- evap/evaluation/management/commands/lint.py | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9b86ef8d14..4a64864fe5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -88,12 +88,12 @@ jobs: - name: Setup python uses: ./.github/setup_python - - name: Run pylint - run: pylint evap -j 0 - - name: Run ruff run: ruff . + - name: Run pylint + run: pylint evap -j 0 + formatter: runs-on: ubuntu-22.04 diff --git a/evap/evaluation/management/commands/lint.py b/evap/evaluation/management/commands/lint.py index b7c4427abb..4e1afede94 100644 --- a/evap/evaluation/management/commands/lint.py +++ b/evap/evaluation/management/commands/lint.py @@ -9,4 +9,7 @@ class Command(BaseCommand): requires_migrations_checks = False def handle(self, *args, **options): + print("Executing ruff .") + subprocess.run(["ruff", "."], check=False) # nosec + print("Executing pylint evap") subprocess.run(["pylint", "evap"], check=False) # nosec From 9a8e1ce6031b32080ca6f2d3ec6c3d129285f407 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 26 Feb 2024 19:49:44 +0100 Subject: [PATCH 49/54] adapt test accordingly --- evap/evaluation/management/commands/lint.py | 4 ++-- evap/evaluation/tests/test_commands.py | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/evap/evaluation/management/commands/lint.py b/evap/evaluation/management/commands/lint.py index 4e1afede94..4f5b5ddbe1 100644 --- a/evap/evaluation/management/commands/lint.py +++ b/evap/evaluation/management/commands/lint.py @@ -9,7 +9,7 @@ class Command(BaseCommand): requires_migrations_checks = False def handle(self, *args, **options): - print("Executing ruff .") + self.stdout.write("Executing ruff .") subprocess.run(["ruff", "."], check=False) # nosec - print("Executing pylint evap") + self.stdout.write("Executing pylint evap") subprocess.run(["pylint", "evap"], check=False) # nosec diff --git a/evap/evaluation/tests/test_commands.py b/evap/evaluation/tests/test_commands.py index b419a208d2..07673606d7 100644 --- a/evap/evaluation/tests/test_commands.py +++ b/evap/evaluation/tests/test_commands.py @@ -4,7 +4,7 @@ from datetime import date, datetime, timedelta from io import StringIO from itertools import chain, cycle -from unittest.mock import call, patch +from unittest.mock import MagicMock, call, patch from django.conf import settings from django.core import mail, management @@ -366,11 +366,12 @@ def test_send_text_answer_review_reminder(self): class TestLintCommand(TestCase): - @staticmethod @patch("subprocess.run") - def test_pylint_called(mock_subprocess_run): - management.call_command("lint") - mock_subprocess_run.assert_called_once_with(["pylint", "evap"], check=False) + def test_pylint_called(self, mock_subprocess_run: MagicMock): + management.call_command("lint", stdout=StringIO()) + self.assertEqual(mock_subprocess_run.call_count, 2) + mock_subprocess_run.assert_any_call(["ruff", "."], check=False) + mock_subprocess_run.assert_any_call(["pylint", "evap"], check=False) class TestFormatCommand(TestCase): From a4edc178be1814db771d5760a94b7742c961a8b3 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 26 Feb 2024 20:06:20 +0100 Subject: [PATCH 50/54] update ruff to latest and migrate settings --- pyproject.toml | 4 +++- requirements-dev.txt | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index fb740cf494..04bf9cdd4d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,6 +25,8 @@ extend_skip_glob = ["**/migrations/*"] [tool.ruff] target-version = "py310" line-length = 120 + +[tool.ruff.lint] # Those are interesting, but not yet addressed: PYI,Q,SIM,PTH,TRY,RUF select = ["F", "E", "B", "W", "N", "UP", "YTT", "FIX", "ASYNC", "A", "DJ", "EXE", "ICN", "G", "SLOT", "TID", "TCH", "INT", "C4", "ISC", "INP", "PIE", "RSE", "RET", "COM","PGH","FLY", "PERF", "PL", "BLE"] ignore = [ @@ -41,7 +43,7 @@ ignore = [ ignore-init-module-imports = true pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert methods use camelCase; Formsets use PascalCase -[tool.ruff.per-file-ignores] +[tool.ruff.lint.per-file-ignores] "**/migrations/*.py" = ["N806"] # migrations have model classes as local variables, we use PascalCase for these "**/commands/*.py" = ["T201"] # printing in commands is ok "deployment/*.py" = ["INP001"] # not intended for direct import diff --git a/requirements-dev.txt b/requirements-dev.txt index f8129dd52f..75e22e6a17 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -10,7 +10,7 @@ mypy~=1.8.0 openpyxl-stubs~=0.1.25 pylint-django~=2.5.4 pylint~=3.0.1 -ruff==0.1.14 +ruff==0.2.2 tblib~=3.0.0 xlrd~=2.0.1 typeguard~=4.1.5 From 2e9518cb1e5a1123eedd08a58ec9c01cd3e566a6 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 4 Mar 2024 19:38:00 +0100 Subject: [PATCH 51/54] do not print in commands, use stdout instead! --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 378ac06799..6904f399e4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,7 +45,6 @@ pep8-naming.extend-ignore-names = ["assert*", "*Formset"] # custom assert metho [tool.ruff.lint.per-file-ignores] "**/migrations/*.py" = ["N806"] # migrations have model classes as local variables, we use PascalCase for these -"**/commands/*.py" = ["T201"] # printing in commands is ok "deployment/*.py" = ["INP001"] # not intended for direct import [tool.ruff.format] From 2a9ccf1878cfdd2ed6b1ba38670c47efc75a77aa Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 8 Apr 2024 18:52:31 +0200 Subject: [PATCH 52/54] update ruff --- requirements-dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 90968a23fc..e971177fdb 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -10,7 +10,7 @@ mypy~=1.8.0 openpyxl-stubs~=0.1.25 pylint-django~=2.5.4 pylint~=3.1.0 -ruff==0.2.2 +ruff==0.3.5 tblib~=3.0.0 xlrd~=2.0.1 typeguard~=4.1.5 From cceca4cc621bf00193780e167e69b0b309448135 Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 8 Apr 2024 19:44:49 +0200 Subject: [PATCH 53/54] lint enrollment_preprocessor --- evap/evaluation/management/commands/lint.py | 2 +- tools/enrollment_preprocessor.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/evap/evaluation/management/commands/lint.py b/evap/evaluation/management/commands/lint.py index 4a07bf294a..f5045417f0 100644 --- a/evap/evaluation/management/commands/lint.py +++ b/evap/evaluation/management/commands/lint.py @@ -10,6 +10,6 @@ class Command(BaseCommand): def handle(self, *args, **options): self.stdout.write("Executing ruff .") - subprocess.run(["ruff", "."], check=False) # nosec + subprocess.run(["ruff", "check", "."], check=False) # nosec self.stdout.write("Executing pylint evap") subprocess.run(["pylint", "evap", "tools"], check=False) # nosec diff --git a/tools/enrollment_preprocessor.py b/tools/enrollment_preprocessor.py index 114eaec06a..fc14faa7dc 100755 --- a/tools/enrollment_preprocessor.py +++ b/tools/enrollment_preprocessor.py @@ -8,7 +8,8 @@ from io import BytesIO from itertools import chain from pathlib import Path -from typing import Iterator, NamedTuple, TextIO +from typing import NamedTuple, TextIO +from collections.abc import Iterator from openpyxl import Workbook, load_workbook from openpyxl.cell import Cell From aa4da6b1357d8ab074b3cfa9c2ea1aa26deb334d Mon Sep 17 00:00:00 2001 From: Jonas Dittrich <58814480+Kakadus@users.noreply.github.com> Date: Mon, 8 Apr 2024 20:29:15 +0200 Subject: [PATCH 54/54] sort imports --- tools/enrollment_preprocessor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/enrollment_preprocessor.py b/tools/enrollment_preprocessor.py index fc14faa7dc..df459cab13 100755 --- a/tools/enrollment_preprocessor.py +++ b/tools/enrollment_preprocessor.py @@ -4,12 +4,12 @@ import sys from argparse import ArgumentParser from collections import defaultdict +from collections.abc import Iterator from datetime import datetime from io import BytesIO from itertools import chain from pathlib import Path from typing import NamedTuple, TextIO -from collections.abc import Iterator from openpyxl import Workbook, load_workbook from openpyxl.cell import Cell