Replace zip_choices with strict zip (#2064)

Author: Kakadus

evap/evaluation/models.py

@@ -2,10 +2,10 @@
 import secrets
 import uuid
 from collections import defaultdict
+from dataclasses import dataclass
 from datetime import date, datetime, timedelta
 from enum import Enum, auto
-from numbers import Number
-from typing import NamedTuple
+from numbers import Real
 
 from django.conf import settings
 from django.contrib import messages
@@ -1236,25 +1236,23 @@ def can_have_textanswers(self):
         return self.is_text_question or self.is_rating_question and self.allows_additional_textanswers
 
 
-# Let's deduplicate the fields here once mypy is smart enough to keep up with us :)
-class Choices(NamedTuple):
+@dataclass
+class Choices:
     css_class: str
-    values: tuple[Number]
+    values: tuple[Real]
     colors: tuple[str]
-    grades: tuple[Number]
+    grades: tuple[Real]
     names: list[StrOrPromise]
     is_inverted: bool
 
+    def as_name_color_value_tuples(self):
+        return zip(self.names, self.colors, self.values, strict=True)
 
-class BipolarChoices(NamedTuple):
-    css_class: str
-    values: tuple[Number]
-    colors: tuple[str]
-    grades: tuple[Number]
-    names: list[StrOrPromise]
+
+@dataclass
+class BipolarChoices(Choices):
     plus_name: StrOrPromise
     minus_name: StrOrPromise
-    is_inverted: bool
 
 
 NO_ANSWER = 6

evap/evaluation/templatetags/evaluation_filters.py

@@ -5,6 +5,7 @@
 from django.utils.translation import gettext_lazy as _
 
 from evap.evaluation.models import BASE_UNIPOLAR_CHOICES, Contribution, Evaluation
+from evap.results.tools import RatingResult
 from evap.rewards.tools import can_reward_points_be_used_by
 from evap.student.forms import HeadingField
 
@@ -76,12 +77,7 @@
 
 @register.filter(name="zip")
 def _zip(a, b):
-    return zip(a, b)
-
-
-@register.filter()
-def zip_choices(counts, choices):
-    return zip(counts, choices.names, choices.colors, choices.values)
+    return zip(a, b, strict=True)
 
 
 @register.filter
@@ -115,12 +111,12 @@ def percentage_one_decimal(fraction, population):
 
 
 @register.filter
-def to_colors(choices):
-    if not choices:
+def to_colors(question_result: RatingResult | None):
+    if question_result is None:
         # When displaying the course distribution, there are no associated voting choices.
         # In that case, we just use the colors of a unipolar scale.
-        return BASE_UNIPOLAR_CHOICES["colors"]
-    return choices.colors
+        return BASE_UNIPOLAR_CHOICES["colors"][:-1]
+    return question_result.colors
 
 
 @register.filter

evap/results/templates/distribution_with_grade.html

@@ -5,7 +5,7 @@
     <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 %}
+        {% with colors=question_result|to_colors %}
             {% for value, color in distribution|zip:colors %}
                 {% if value != 0 %}
                     <div class="vote-bg-{{ color }}" style="width: {{ value|percentage_one_decimal:1 }};">&nbsp;</div>

evap/results/templates/evaluation_result_widget.html

@@ -7,7 +7,7 @@
         data-bs-toggle="tooltip" data-bs-placement="left" data-fallback-placement='["left", "bottom"]'
         title="{% spaceless %}{% include 'result_widget_tooltip.html' with question_result=course_or_evaluation.single_result_rating_result weight_info=course_or_evaluation|weight_info %}{% endspaceless %}"
     >
-        {% include "distribution_with_grade.html" with distribution=course_or_evaluation.distribution average=course_or_evaluation.avg_grade weight_info=course_or_evaluation|weight_info %}
+        {% include "distribution_with_grade.html" with distribution=course_or_evaluation.distribution average=course_or_evaluation.avg_grade weight_info=course_or_evaluation|weight_info question_result=None %}
     </div>
 {% else %}
     <div class="d-flex" data-bs-toggle="tooltip" data-bs-placement="left" title="{% trans 'Not enough answers were given.' %}">

evap/results/templates/result_widget_tooltip.html

@@ -13,7 +13,7 @@
     {% endif %}
     {% with question_result.question.is_bipolar_likert_question as is_bipolar %}
         <p>
-            {% for count, name, color, value in question_result.counts|zip_choices:question_result.choices %}
+            {% for count, name, color, value in question_result.zipped_choices %}
                 <span class='vote-text-{{ color }} fas fa-fw-absolute
                     {% if is_bipolar and value < 0 %}fa-caret-down pole-icon
                     {% elif is_bipolar and value > 0 %}fa-caret-up pole-icon

evap/results/tools.py

@@ -58,23 +58,31 @@ def is_published(cls, rating_result) -> TypeGuard["PublishedRatingResult"]:
     def has_answers(cls, rating_result) -> TypeGuard["AnsweredRatingResult"]:
         return isinstance(rating_result, AnsweredRatingResult)
 
-    def __init__(self, question, additional_text_result=None):
+    def __init__(self, question, additional_text_result=None) -> None:
         assert question.is_rating_question
         self.question = discard_cached_related_objects(copy(question))
         self.additional_text_result = additional_text_result
+        self.colors = tuple(
+            color for _, color, value in self.choices.as_name_color_value_tuples() if value != NO_ANSWER
+        )
 
     @property
     def choices(self):
         return CHOICES[self.question.type]
 
 
 class PublishedRatingResult(RatingResult):
-    def __init__(self, question, answer_counters, additional_text_result=None):
+    def __init__(self, question, answer_counters, additional_text_result=None) -> None:
         super().__init__(question, additional_text_result)
-        counts = OrderedDict((value, 0) for value in self.choices.values if value != NO_ANSWER)
+        counts = OrderedDict(
+            (value, [0, name, color, value]) for (name, color, value) in self.choices.as_name_color_value_tuples()
+        )
+        counts.pop(NO_ANSWER)
         for answer_counter in answer_counters:
-            counts[answer_counter.answer] = answer_counter.count
-        self.counts = tuple(counts.values())
+            assert counts[answer_counter.answer][0] == 0
+            counts[answer_counter.answer][0] = answer_counter.count
+        self.counts = tuple(count for count, _, _, _ in counts.values())
+        self.zipped_choices = tuple(counts.values())
 
     @property
     def count_sum(self) -> int: