Use score summaries in data download - #43
Conversation
3d8ff0c to
023d8cb
Compare
The submissions API has a get_score() method which, given a student item,
will look up the relevant ScoreSummary and return the latest Score it
contains. Our data download should do the same.
To avoid confusion, we have elected to hide scores that could not be
returned from api.get_score(). The other options are:
- include a score that may be for a different submission for the same
student item, if using ScoreSummary.latest in all cases
- include scores that may not "count", because they'll never be the
return value of api.get_score()
Test has been updated.
023d8cb to
8d14bab
Compare
| submission_qs = _use_read_replica(submission_qs) | ||
|
|
||
| query = submission_qs.select_related('student_item').prefetch_related('score_set').filter( | ||
| query = submission_qs.select_related('student_item__scoresummary__latest__submission').filter( |
There was a problem hiding this comment.
This select_related means that, for each submission in our query, we are fetching:
- its student_item, which can be tied to a
- scoresummary, which can be tied to a
- score (latest), which can be tied to a
- submission
The upshot is that, for each submission, we can ask the question If we tried to get the score for this submission's student_item, would the score reported be associated with this submission? If it is, the score is included in our data download. If it is not, no score is returned.
This is to handle a situation like this one:
- student makes a submission
- student gets a poor score on that submission
- student makes a new submission
- student gets a better score on that submission
The returned values in this case would be both submissions, but no score would be reported for the first one. Is this desirable? Or would it be better to return both scores even though one of them doesn't count? We have elected to go this route for now, since that seems to be how the api is set up for get_score. The third option (that I really don't like) is to include only the relevant score for each submission's student_item, but then in the scenario above you'd end up with a row that seems to indicate the first submission got the second score.
Worth noting: ORA imposes a limit of 1 submission per student item (excluding delete state requests, but that data was already being excluded), so this is irrelevant in the context of ORA data downloads. It's more a question of how edx-submissions wants to handle this theoretical case for other consumers in the future.
There was a problem hiding this comment.
As you stated, it really is a question of who needs the data download and what they want included:
- Do they want all scores with additional data (like a latest flag) to help them filter the data in the download appropriately? Or,
- Does someone just want the latest (i.e. relevant) scores with no other historical scores?
One possibility is to go with the latest, since it is simpler, and add an option later if and when it is needed. Can we find out if this aligns with the original requirement at a minimum?
@ormsbee @scottrish Thoughts on this?
Depending on which way we go, I'll have some additional minor requests on naming and commenting to make this more clear.
There was a problem hiding this comment.
If we stick with latest score, the comment for this method should mention "latest score" each time it currently reads "score". The function comment should also mention that an empty score will be sent for submissions with no score.
There was a problem hiding this comment.
My guess is that most people will only want the latest relevant score with no historical data for something like a download, so that should be the default behavior. But I think there are valid reasons for wanting every submission and its corresponding score or scores in the future.
|
@dianakhuang Here's what I have for only including relevant scores in the data download. @robrap Could you check over my comment above, and see if you agree with the route we're taking? |
|
👍 |
|
I don't want to close off this discussion, but we have the loadtest environment available today and would like to try this code out to get some ballpark figures. I'm going to merge this into Diana's branch, but the question of "only latest/relevant scores" vs. "all scores with flag" is still open. If we do end up going with "all scores and a flag", the performance will probably be slightly worse, but not terribly so. |
Use score summaries in data download
|
|
||
| # We only include the score for a given submission if it is not a reset score and it "counts", that is, | ||
| # if it is the latest score on the score summary tracking the submission's student_item. This matches the | ||
| # behavior of the API's get_score method. |
There was a problem hiding this comment.
@efischer19 @dianakhuang Does the following comment sound more clear to you?
# Only include the score if it is not a reset score (is_hidden), and if the current submission matches
# the latest score's submission. This matches the behavior of the API's get_score method.
There was a problem hiding this comment.
It's better, but I'm still looking at if the current submission matches the latest score's submission. Is there some way we can mention the student_item in there as well? Something like if the current submission corresponds to the latest score for this student_item?
There was a problem hiding this comment.
I'd like something that makes all the lines of code more clear, including the last test comparing the submission.uuid's.
Maybe:
...if the current submission is the same as the student_item's latest score's submission.
There was a problem hiding this comment.
👍 I like that one, I'll implement it when I update this code later. Thanks!
4 participants
Let's see how this goes