Average grades in export #2366
Average grades in export #2366
Conversation
fidoriel
force-pushed
the
average-grades-in-export
branch
3 times, most recently
from
e5117a8 to
d51385d
Compare
fidoriel
force-pushed
the
average-grades-in-export
branch
from
d51385d to
af9697b
Compare
fidoriel
force-pushed
the
average-grades-in-export
branch
from
af9697b to
382680b
Compare
evap/results/exporters.py
Outdated
| @@ -198,6 +203,8 @@ def write_headings_and_evaluation_info( | |||
| else: | |||
| self.write_cell(export_name, "headline") | |||
|
|
|||
| self.write_cell("Average for this Question", "evaluation") | |||
There was a problem hiding this comment.
I think "Average for this Question" would need to be translated here.
The styles used here and in the lines changed below confuse me. Why is this using "evaluation"? Why is the empty cell in the course type column using the "program" style below?
Please double-check that these all make sense, and if we really need some unintuitive value, add some explanation as to why.
There was a problem hiding this comment.
This is still not marked for translation, is it? I would also prefer to have something like "Average for this question over all evaluations in all published semesters" and please make it italic to distinguish it from the course names.
fidoriel
force-pushed
the
average-grades-in-export
branch
from
dced350 to
6163186
Compare
|
|
||
| self.assertEqual( | ||
| float(workbook.sheets()[0].row_values(5)[1]), | ||
| (float(workbook.sheets()[0].row_values(5)[2]) + float(workbook.sheets()[0].row_values(5)[3])) / 2, |
There was a problem hiding this comment.
From a quick glance, it looks like the setup above creates the same exact average result for both evaluations and same count of answers inside of them. This doesn't test whether we compute the average-of-averages with correct weighting. Can we change them to have different answer counts?
Coincidentally, float-equality here made me wonder about correctness. It should work if the two values here are identical, the sum is then 2 * the original value, which is divisible by 2 to perfectly recreate the original value. However, I'd like the test to test a somewhat more sophisticated case. We'll likely need to use assertAlmostEqual then.
| self.assertEqual(float(workbook.sheets()[0].row_values(8)[1]), float(workbook.sheets()[0].row_values(8)[3])) | ||
| self.assertEqual("", workbook.sheets()[0].row_values(8)[2]) |
There was a problem hiding this comment.
I may be tired, but I don't understand what we're asserting here. What is the intention here? Can we maybe improve the test name or add a comment?
evap/results/exporters.py
Outdated
| @@ -198,6 +203,8 @@ def write_headings_and_evaluation_info( | |||
| else: | |||
| self.write_cell(export_name, "headline") | |||
|
|
|||
| self.write_cell("Average for this Question", "evaluation") | |||
There was a problem hiding this comment.
This is still not marked for translation, is it? I would also prefer to have something like "Average for this question over all evaluations in all published semesters" and please make it italic to distinguish it from the course names.
closes #2001