diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 544304bc019e2ff523ea14d2d02db82897f3ff30..348ed9ea457f89921aa22e3f37ddc575b71daab2 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -47,6 +47,24 @@ jobs: run: ${{ matrix.command }} + mypy: + runs-on: ubuntu-18.04 + + container: + image: python:3.7 + + name: MyPy + + steps: + - name: Check out repository code + uses: actions/checkout@v2 + - name: Install dependencies + run: pip install -r requirements-dev.txt + - name: Add localsettings + run: cp evap/settings_test.py evap/localsettings.py + - name: Run MyPy + run: mypy -p evap + linter: runs-on: ubuntu-18.04 diff --git a/evap/evaluation/management/commands/format.py b/evap/evaluation/management/commands/format.py index f513276cf2f95d061b28617e5c8cb9169f9d591d..b4ea14f13b8f8c99c94b417992f1cccd0e0053d0 100644 --- a/evap/evaluation/management/commands/format.py +++ b/evap/evaluation/management/commands/format.py @@ -5,7 +5,7 @@ from django.core.management.base import BaseCommand class Command(BaseCommand): args = "" - help = "Runs the code formatter" + help = "Runs code formatting" requires_migrations_checks = False def handle(self, *args, **options): diff --git a/evap/evaluation/management/commands/precommit.py b/evap/evaluation/management/commands/precommit.py index e147efa7185477aef1734bb10d3a5b2997e631a7..455b254c8eb13d2c44ba4821ed6bc462fbaf7d75 100644 --- a/evap/evaluation/management/commands/precommit.py +++ b/evap/evaluation/management/commands/precommit.py @@ -16,6 +16,8 @@ class Command(BaseCommand): print("Please call me from the evap root directory (where manage.py resides)") sys.exit(1) + call_command("typecheck") + # subprocess call so our sys.argv check in settings.py works subprocess.run(["./manage.py", "test"], check=False) # nosec diff --git a/evap/evaluation/management/commands/typecheck.py b/evap/evaluation/management/commands/typecheck.py new file mode 100644 index 0000000000000000000000000000000000000000..080ad6b456b396d31149cb1288b045a813120a9b --- /dev/null +++ b/evap/evaluation/management/commands/typecheck.py @@ -0,0 +1,12 @@ +import subprocess # nosec + +from django.core.management.base import BaseCommand + + +class Command(BaseCommand): + args = "" + help = "Runs the type checker (mypy)" + requires_migrations_checks = False + + def handle(self, *args, **options): + subprocess.run(["mypy", "-p", "evap"], check=True) # nosec diff --git a/evap/evaluation/migrations/0019_merge.py b/evap/evaluation/migrations/0019_merge.py index 573b79cf8bd190e49e1b6418c484edceb63019ab..4e737b6399fa8771aedbefafca9aff6c592da6d8 100644 --- a/evap/evaluation/migrations/0019_merge.py +++ b/evap/evaluation/migrations/0019_merge.py @@ -7,6 +7,3 @@ class Migration(migrations.Migration): ('evaluation', '0018_unique_degrees'), ('evaluation', '0014_rename_lecturer'), ] - - operations = [ - ] diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index 60cb31e9a4367d054e71630745fecbba72e07404..6029639ca5589e0822b65999fb37b0d1da1858ef 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -1,9 +1,11 @@ import logging import secrets import uuid -from collections import defaultdict, namedtuple +from collections import defaultdict from datetime import date, datetime, timedelta from enum import Enum, auto +from numbers import Number +from typing import Dict, List, NamedTuple, Tuple, Union from django.conf import settings from django.contrib import messages @@ -17,7 +19,7 @@ from django.db.models import Count, Manager, OuterRef, Q, Subquery from django.db.models.functions import Coalesce from django.dispatch import Signal, receiver from django.template import Context, Template -from django.template.base import TemplateSyntaxError +from django.template.exceptions import TemplateSyntaxError from django.template.loader import render_to_string from django.urls import reverse from django.utils.functional import cached_property @@ -1178,32 +1180,54 @@ class Question(models.Model): return self.is_text_question or self.is_rating_question and self.allows_additional_textanswers -Choices = namedtuple("Choices", ("cssClass", "values", "colors", "grades", "names")) -BipolarChoices = namedtuple("BipolarChoices", Choices._fields + ("plus_name", "minus_name")) +# Let's deduplicate the fields here once mypy is smart enough to keep up with us :) +Choices = NamedTuple( + "Choices", + [ + ("css_class", str), + ("values", Tuple[Number]), + ("colors", Tuple[str]), + ("grades", Tuple[Number]), + ("names", List[str]), + ], +) +BipolarChoices = NamedTuple( + "BipolarChoices", + [ + ("css_class", str), + ("values", Tuple[Number]), + ("colors", Tuple[str]), + ("grades", Tuple[Number]), + ("names", List[str]), + ("plus_name", str), + ("minus_name", str), + ], +) + NO_ANSWER = 6 BASE_UNIPOLAR_CHOICES = { - "cssClass": "vote-type-unipolar", + "css_class": "vote-type-unipolar", "values": (1, 2, 3, 4, 5, NO_ANSWER), "colors": ("green", "lime", "yellow", "orange", "red", "gray"), "grades": (1, 2, 3, 4, 5), } BASE_BIPOLAR_CHOICES = { - "cssClass": "vote-type-bipolar", + "css_class": "vote-type-bipolar", "values": (-3, -2, -1, 0, 1, 2, 3, NO_ANSWER), "colors": ("red", "orange", "lime", "green", "lime", "orange", "red", "gray"), "grades": (5, 11 / 3, 7 / 3, 1, 7 / 3, 11 / 3, 5), } BASE_YES_NO_CHOICES = { - "cssClass": "vote-type-yes-no", + "css_class": "vote-type-yes-no", "values": (1, 5, NO_ANSWER), "colors": ("green", "red", "gray"), "grades": (1, 5), } -CHOICES = { +CHOICES: Dict[int, Union[Choices, BipolarChoices]] = { Question.LIKERT: Choices( names=[ _("Strongly\nagree"), @@ -1213,7 +1237,7 @@ CHOICES = { _("Strongly\ndisagree"), _("No answer"), ], - **BASE_UNIPOLAR_CHOICES, + **BASE_UNIPOLAR_CHOICES, # type: ignore ), Question.GRADE: Choices( names=[ @@ -1224,7 +1248,7 @@ CHOICES = { "5", _("No answer"), ], - **BASE_UNIPOLAR_CHOICES, + **BASE_UNIPOLAR_CHOICES, # type: ignore ), Question.EASY_DIFFICULT: BipolarChoices( minus_name=_("Easy"), @@ -1239,7 +1263,7 @@ CHOICES = { _("Way too\ndifficult"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, + **BASE_BIPOLAR_CHOICES, # type: ignore ), Question.FEW_MANY: BipolarChoices( minus_name=_("Few"), @@ -1254,7 +1278,7 @@ CHOICES = { _("Way too\nmany"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, + **BASE_BIPOLAR_CHOICES, # type: ignore ), Question.LITTLE_MUCH: BipolarChoices( minus_name=_("Little"), @@ -1269,7 +1293,7 @@ CHOICES = { _("Way too\nmuch"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, + **BASE_BIPOLAR_CHOICES, # type: ignore ), Question.SMALL_LARGE: BipolarChoices( minus_name=_("Small"), @@ -1284,7 +1308,7 @@ CHOICES = { _("Way too\nlarge"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, + **BASE_BIPOLAR_CHOICES, # type: ignore ), Question.SLOW_FAST: BipolarChoices( minus_name=_("Slow"), @@ -1299,7 +1323,7 @@ CHOICES = { _("Way too\nfast"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, + **BASE_BIPOLAR_CHOICES, # type: ignore ), Question.SHORT_LONG: BipolarChoices( minus_name=_("Short"), @@ -1314,7 +1338,7 @@ CHOICES = { _("Way too\nlong"), _("No answer"), ], - **BASE_BIPOLAR_CHOICES, + **BASE_BIPOLAR_CHOICES, # type: ignore ), Question.POSITIVE_YES_NO: Choices( names=[ @@ -1322,7 +1346,7 @@ CHOICES = { _("No"), _("No answer"), ], - **BASE_YES_NO_CHOICES, + **BASE_YES_NO_CHOICES, # type: ignore ), Question.NEGATIVE_YES_NO: Choices( names=[ @@ -1330,7 +1354,7 @@ CHOICES = { _("Yes"), _("No answer"), ], - **BASE_YES_NO_CHOICES, + **BASE_YES_NO_CHOICES, # type: ignore ), } @@ -1517,7 +1541,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin): verbose_name_plural = _("users") USERNAME_FIELD = "email" - REQUIRED_FIELDS = [] + REQUIRED_FIELDS: List[str] = [] objects = UserProfileManager() diff --git a/evap/evaluation/templatetags/navbar_templatetags.py b/evap/evaluation/templatetags/navbar_templatetags.py index 332c6d337395572137252a3e7d7798b30140ecf6..ece4b251aafcb6c07f9e8bbe9032c96f4541ed82 100644 --- a/evap/evaluation/templatetags/navbar_templatetags.py +++ b/evap/evaluation/templatetags/navbar_templatetags.py @@ -1,7 +1,7 @@ from django.template import Library from evap.evaluation.models import Semester -from evap.settings import DEBUG, LANGUAGES +from evap.settings import DEBUG, LANGUAGES # type: ignore register = Library() diff --git a/evap/evaluation/tests/test_commands.py b/evap/evaluation/tests/test_commands.py index 128341a34e5b8c3c946c9c27a14b393a96b8e2ff..904880fdae754d446044f88450b636b3ff6f7172 100644 --- a/evap/evaluation/tests/test_commands.py +++ b/evap/evaluation/tests/test_commands.py @@ -383,6 +383,14 @@ class TestFormatCommand(TestCase): ) +class TestTypecheckCommand(TestCase): + @patch("subprocess.run") + def test_mypy_called(self, mock_subprocess_run): + management.call_command("typecheck") + self.assertEqual(len(mock_subprocess_run.mock_calls), 1) + mock_subprocess_run.assert_has_calls([call(["mypy", "-p", "evap"], check=True)]) + + class TestPrecommitCommand(TestCase): @patch("subprocess.run") @patch("evap.evaluation.management.commands.precommit.call_command") @@ -391,6 +399,7 @@ class TestPrecommitCommand(TestCase): mock_subprocess_run.assert_called_with(["./manage.py", "test"], check=False) - self.assertEqual(mock_call_command.call_count, 2) + self.assertEqual(mock_call_command.call_count, 3) + mock_call_command.assert_any_call("typecheck") mock_call_command.assert_any_call("lint") mock_call_command.assert_any_call("format") diff --git a/evap/evaluation/tests/tools.py b/evap/evaluation/tests/tools.py index 38a8dadb63e48514077241056494d6606767213e..4466b48cd7f90fd5dc0b02de6aa25c1decba9f01 100644 --- a/evap/evaluation/tests/tools.py +++ b/evap/evaluation/tests/tools.py @@ -1,6 +1,7 @@ import functools import os from datetime import timedelta +from typing import List, Union from django.conf import settings from django.contrib.auth.models import Group @@ -84,7 +85,7 @@ def render_pages(test_item): class WebTestWith200Check(WebTest): url = "/" - test_users = [] + test_users: List[Union[UserProfile, str]] = [] def test_check_response_code_200(self): for user in self.test_users: diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index 41bdd1a73f291e100414e6dd6a833ab86b0181f4..4699b06da831fc2967a51f35e47aad161eaee43a 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -1,5 +1,6 @@ import datetime from abc import ABC, abstractmethod +from typing import Optional from urllib.parse import quote import xlwt @@ -96,7 +97,7 @@ class ExcelExporter(ABC): # Derived classes can set this to # have a sheet added at initialization. - default_sheet_name = None + default_sheet_name: Optional[str] = None def __init__(self): self.workbook = xlwt.Workbook() diff --git a/evap/results/exporters.py b/evap/results/exporters.py index 59ca75c463c691918ccf81f9f2c2c34642a1e393..027676ab06dbea8b99ced878acfa3e8c3e54b7fb 100644 --- a/evap/results/exporters.py +++ b/evap/results/exporters.py @@ -1,6 +1,7 @@ import warnings from collections import OrderedDict, defaultdict from itertools import chain, repeat +from typing import Dict, Tuple import xlwt from django.db.models import Q @@ -24,7 +25,7 @@ class ResultsExporter(ExcelExporter): STEP = 0.2 # we only have a limited number of custom colors # Filled in ResultsExporter.init_grade_styles - COLOR_MAPPINGS = {} + COLOR_MAPPINGS: Dict[int, Tuple[int, int, int]] = {} styles = { "evaluation": xlwt.easyxf( diff --git a/evap/results/templates/distribution_with_grade.html b/evap/results/templates/distribution_with_grade.html index 221438c27b7c39bed7bfcad2e69ad1f6bc4072b2..9ae33a07406c54e42329bc829979b16164b31d69 100644 --- a/evap/results/templates/distribution_with_grade.html +++ b/evap/results/templates/distribution_with_grade.html @@ -2,7 +2,7 @@ {% load evaluation_filters %} <div class="distribution-bar-container{% if question_result.warning %} participants-warning{% endif %}"> - <div class="distribution-bar {{ question_result.choices.cssClass }}" + <div class="distribution-bar {{ question_result.choices.css_class }}" {% if question_result.question.is_bipolar_likert_question %} style="left: {{ question_result.minus_balance_count|percentage_one_decimal:question_result.count_sum }}"{% endif %} > {% with colors=question_result.choices|to_colors %} diff --git a/evap/results/tools.py b/evap/results/tools.py index 681bb50c9a0a1d8445d7a560016f7505ecfdfef8..4cd3633defe89cf7a9244dabb881935da9529930 100644 --- a/evap/results/tools.py +++ b/evap/results/tools.py @@ -1,5 +1,6 @@ from collections import OrderedDict, defaultdict, namedtuple from math import ceil, modf +from typing import Tuple, cast from django.conf import settings from django.core.cache import caches @@ -375,7 +376,9 @@ def distribution_to_grade(distribution): def color_mix(color1, color2, fraction): - return tuple(int(round(color1[i] * (1 - fraction) + color2[i] * fraction)) for i in range(3)) + return cast( + Tuple[int, int, int], tuple(int(round(color1[i] * (1 - fraction) + color2[i] * fraction)) for i in range(3)) + ) def get_grade_color(grade): diff --git a/evap/results/views.py b/evap/results/views.py index 84a03d67cd84f1b969942f76308f2ba5e9f1abdb..a4f57beb283ee8a69a158e808d0b6200ae164d0b 100644 --- a/evap/results/views.py +++ b/evap/results/views.py @@ -112,7 +112,7 @@ def update_template_cache_of_published_evaluations_in_course(course): def get_evaluations_with_prefetched_data(evaluations): - if isinstance(evaluations, QuerySet): + if isinstance(evaluations, QuerySet): # type: ignore evaluations = evaluations.select_related("course__type").prefetch_related( "course__degrees", "course__semester", diff --git a/evap/settings.py b/evap/settings.py index 37ce6a5b6d51dea43d48e863366d618e400886cf..fea0244ceb91e90272efabc79704e88d2c0f17ef 100644 --- a/evap/settings.py +++ b/evap/settings.py @@ -1,3 +1,5 @@ +# type: ignore + """ Django settings for EvaP project. diff --git a/evap/staff/forms.py b/evap/staff/forms.py index b30553706db30bf0c213b9c857b6e9ef93203f93..8dae2b034ce4d210976f120f86390644bb3d356e 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -250,7 +250,7 @@ class CourseFormMixin: self.add_error(name_field, e) -class CourseForm(CourseFormMixin, forms.ModelForm): +class CourseForm(CourseFormMixin, forms.ModelForm): # type: ignore semester = forms.ModelChoiceField(Semester.objects.all(), disabled=True, required=False, widget=forms.HiddenInput()) def __init__(self, *args, **kwargs): @@ -260,17 +260,17 @@ class CourseForm(CourseFormMixin, forms.ModelForm): disable_all_fields(self) -class CourseCopyForm(CourseFormMixin, forms.ModelForm): +class CourseCopyForm(CourseFormMixin, forms.ModelForm): # type: ignore 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) field_order = ["semester"] - def __init__(self, data=None, instance: Course = None): + def __init__(self, data=None, *, instance: Course): self.old_course = instance - opts = self._meta - initial = forms.models.model_to_dict(instance, opts.fields, opts.exclude) + opts = self._meta # type: ignore + initial = forms.models.model_to_dict(instance, opts.fields, opts.exclude) # type: ignore super().__init__(data=data, initial=initial) self._set_responsibles_queryset(instance) diff --git a/evap/staff/staff_mode.py b/evap/staff/staff_mode.py index f51c935a4187652985856c6f7a3ef21ece877151..3ac3d50e60b3a8012849600909a5cadc81973340 100644 --- a/evap/staff/staff_mode.py +++ b/evap/staff/staff_mode.py @@ -3,7 +3,7 @@ import time from django.contrib import messages from django.utils.translation import ugettext as _ -from evap.settings import STAFF_MODE_INFO_TIMEOUT, STAFF_MODE_TIMEOUT +from evap.settings import STAFF_MODE_INFO_TIMEOUT, STAFF_MODE_TIMEOUT # type: ignore def staff_mode_middleware(get_response): diff --git a/evap/staff/views.py b/evap/staff/views.py index c205af0edc5cf503830f632f2008b5f1d1f17231..e635405e2beb43439df42e46e04ae2024cba31f3 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -3,7 +3,7 @@ import itertools from collections import OrderedDict, defaultdict, namedtuple from dataclasses import dataclass from datetime import date, datetime -from typing import Any, Container, Dict +from typing import Any, Container, Dict, Optional from django.conf import settings from django.contrib import messages @@ -226,10 +226,10 @@ def semester_view(request, semester_id): class EvaluationOperation: - email_template_name = None - email_template_contributor_name = None - email_template_participant_name = None - confirmation_message = None + email_template_name: Optional[str] = None + email_template_contributor_name: Optional[str] = None + email_template_participant_name: Optional[str] = None + confirmation_message: Optional[str] = None @staticmethod def applicable_to(evaluation): diff --git a/evap/student/templates/student_vote_questionnaire_group.html b/evap/student/templates/student_vote_questionnaire_group.html index d0726384d66692298b1f79cb297ee27a671f77df..0998bcdc8a5090579a5d5e3984af6a915d3d2bdf 100644 --- a/evap/student/templates/student_vote_questionnaire_group.html +++ b/evap/student/templates/student_vote_questionnaire_group.html @@ -43,7 +43,7 @@ {% endif %} {% if field|is_choice_field %} <div class="col-answer col-lg-8 col-xl-7 my-auto d-flex"> - <div class="vote-inputs tab-row {{ field.field.widget.attrs.choices.cssClass }} btn-group" data-bs-toggle="buttons"> + <div class="vote-inputs tab-row {{ field.field.widget.attrs.choices.css_class }} btn-group" data-bs-toggle="buttons"> {% for choice, color in field|zip:field.field.widget.attrs.choices.colors %} <input id="{{ choice.id_for_label }}" class="tab-selectable num-selectable btn-check" name="{{ choice.data.name }}" type="radio" value="{{ choice.data.value }}" autocomplete="off"{% if field.value == choice.data.value %} checked{% endif %}{% if preview %} disabled{% endif %}/> <label for="{{ choice.id_for_label }}" class="btn btn-sm vote-btn vote-btn-{{ color }} d-flex{% if field.value == choice.data.value %} active{% endif %}{% if field.errors %} choice-error{% endif %}"> diff --git a/pyproject.toml b/pyproject.toml index 92902f4b8bf25fe1d6ea5f72d48c5775d7361606..d033c206065922024937f44adad20c8d44b97ca8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,6 +8,8 @@ extend-exclude = """\ .*/migrations/.*\ """ +############################################## + [tool.isort] profile = "black" src_paths = ["evap"] @@ -15,6 +17,8 @@ line_length = 120 skip_gitignore = true extend_skip_glob = ["**/migrations/*"] +############################################## + [tool.pylint.master] jobs = 0 @@ -58,7 +62,7 @@ disable = [ "duplicate-code", # Can not be suppressed for the 4 false positives we have: https://github.com/PyCQA/pylint/issues/214 ] - +############################################## [tool.coverage.run] branch = true @@ -67,3 +71,28 @@ source = ["evap"] [tool.coverage.report] omit = ["*/migrations/*", "*__init__.py"] + +############################################## + +[tool.mypy] +plugins = ["mypy_django_plugin.main"] +exclude = 'evap/.*/migrations/.*\.py$' + +[tool.django-stubs] +django_settings_module = "evap.settings" + +[[tool.mypy.overrides]] +module = [ + "django_fsm.*", + "django_sendfile.*", + "django_webtest.*", + "debug_toolbar.*", + "mozilla_django_oidc.*", + "model_bakery.*", + "xlrd.*", + "xlwt.*", + "xlutils.*", + + "evap.staff.fixtures.*", +] +ignore_missing_imports = true diff --git a/requirements-dev.txt b/requirements-dev.txt index 1daaa81a31c7b29c06a31f5dd3aa0f7e09418e60..908bef7b459466a67fd047fb25250dfbaf121929 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -6,3 +6,5 @@ pylint==2.9.5 pylint-django==2.4.4 black==21.7b0 isort==5.9.3 +mypy +django-stubs