Replace zip_choices with strict zip
#2064
Conversation
Kakadus
changed the title
zip_choices with strict zip
evap/results/tools.py
Outdated
| 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()) | ||
| counts[answer_counter.answer][0] = answer_counter.count | ||
| self.zipped_choices = tuple(counts.values()) | ||
| self.counts = tuple((count for count, _, _, _ in counts.values())) |
There was a problem hiding this comment.
This feels overly complicated, maybe something like the following is better?
answer_indices = {
answer: i for i, answer in enumerate(self.choices.values) if answer != NO_ANSWER
}
counts = [0] * len(answer_indices)
for counter in answer_counters:
counts[answer_indices[counter.answer]] = counter.count
self.counts = tuple(counts)
self.zipped_choices = self.choices.as_name_color_value_tuples()We should probably refactor the whole choices business to use dataclasses instead of named tuples and increase typing with these result classes here, but we don't need to do it now
There was a problem hiding this comment.
The problem is, that self.choices.as_name_color_value_tuples() contains a NO_ANSWER-element as the last element. This needs to be removed from the result of as_name_color_value_tuples. Also, the counts need to be prepended
Kakadus
force-pushed
the
zip-choices-strict
branch
from
f857692 to
e68e179
Compare
Kakadus
force-pushed
the
zip-choices-strict
branch
from
3bd6cad to
2d7868f
Compare
| {% with colors=question_result.choices|to_colors %} | ||
| {% with colors=question_result|to_colors %} | ||
| {% for value, color in distribution|zip:colors %} |
There was a problem hiding this comment.
to your other comment: before, distribution values (w/o NO_ANSWER) may have been zipped with the colors with gray for NO_ANSWER.
Now, I explicitly remove NO_ANSWER from the colors and zip them strictly. I don't think a test is necessary, because this is already covered and only does some narrowing on the zipable tuples
There was a problem hiding this comment.
Looking good so far, but will have to recheck changes to the Result classes once #2091 is merged
rewrite choices logic cleanup revert counters logic improvement merge Choices with CHoicesBase zip QuestionResults colors reformat
Kakadus
force-pushed
the
zip-choices-strict
branch
from
c2cc3bd to
ad9bb22
Compare
niklasmohrin
changed the title
zip_choices with strict zipzip_choices with strict zip
Another commit extracted for #2051. The
zip_choicesfilter zipped choices'countsandnames. This asserts, that the no answer option is always the last option.