Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add weight fields to submission divider #221

Merged
merged 22 commits into from Oct 4, 2017

Conversation

@olmokramer
Copy link
Collaborator

commented Sep 17, 2017

Description

Add weight fields to the submission divider so submissions are not necessarily divided equally over all TAs.

  • Add support in the backend, probably by replacing the divided field with a weight field
  • Initialise everyone at 0, and enable the checkbox next to someone's name when their weight changes
  • Fix percentage calculation

Closes #206

Screenshot or -recording (if applicable)

The percentages are still wrong, but this is the general idea.
codegrade-divide

@ghost ghost assigned olmokramer Sep 17, 2017

@ghost ghost added the in progress label Sep 17, 2017

@codecov

This comment has been minimized.

Copy link

commented Sep 17, 2017

Codecov Report

Merging #221 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #221      +/-   ##
==========================================
+ Coverage   98.51%   98.56%   +0.05%     
==========================================
  Files          22       22              
  Lines        2553     2643      +90     
==========================================
+ Hits         2515     2605      +90     
  Misses         38       38
Impacted Files Coverage Δ
psef/models.py 98.81% <100%> (+0.14%) ⬆️
psef/v1/assignments.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a9ebef...3ed623b. Read the comment docs.

@libre-man

This comment has been minimized.

Copy link
Collaborator

commented Sep 18, 2017

The first point needs some discussion. We don't save who is divided at the moment, we simply get this information from other information in the database. I would prefer that this would stay the same, so this would result in a query that is not trivial.

@ghost ghost assigned libre-man Sep 18, 2017

libre-man added 6 commits Sep 21, 2017

@libre-man libre-man force-pushed the assign-ratio-to-auto-divide branch from 1eee874 to 6e96fc5 Sep 26, 2017

libre-man added 4 commits Sep 26, 2017

@libre-man libre-man changed the title WIP: Add weight fields to submission divider Add weight fields to submission divider Sep 26, 2017

@libre-man

This comment has been minimized.

Copy link
Collaborator

commented Sep 26, 2017

I think this should be mergeable by now. However there is still the problem with how to handle weights and assignee overrides. I think it should not change the weights, however what would be nice if there was an indication in the course admin interface that an override was done. However how should this be handled when doing a reassign?

@olmokramer

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 29, 2017

I think it's fine if we do nothing when we reassign a submission, the weights make sense when automatically dividing submissions but not so much when you want to manually override them.

@olmokramer
Copy link
Collaborator Author

left a comment

I'd prefer the percentage text to be right aligned, but other than that nice job! Can't approve as I started this PR, so if you change the percentage text alignment you can approve and merge it.

@olmokramer

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 29, 2017

Ah I found an issue I think: I divided an assignment with 3 submissions to 2 TAs with equal weights. Then I removed the assignee from a submission, so now both assignees had 1 submission and 1 submission was unassigned. When I clicked the "Divide" button again I expected the unassigned submission to be assigned again, but stayed unassigned.

@olmokramer

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 29, 2017

Oh, and maybe there should be an explanation somewhere that the numbers are used as weights for the division (and not percentages or number of submissions).

@olmokramer

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 2, 2017

Please make the weight field a float. I know you don't like it but there's little reason for it not to be a float.

olmokramer and others added 2 commits Oct 2, 2017
libre-man and others added 2 commits Oct 2, 2017
@libre-man
Copy link
Collaborator

left a comment

Approve on the behalf of @olmokramer.

libre-man added 2 commits Oct 2, 2017
libre-man added 3 commits Oct 2, 2017

@libre-man libre-man merged commit 9dabfc2 into master Oct 4, 2017

5 checks passed

codecov/patch 100% of diff hit (target 98.51%)
Details
codecov/project 98.56% (+0.05%) compared to 9a9ebef
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.05%) to 98.562%
Details
pyup.io/safety-ci No dependencies with known security vulnerabilities.
Details

@ghost ghost removed the in progress label Oct 4, 2017

@libre-man libre-man deleted the assign-ratio-to-auto-divide branch Oct 4, 2017

@olmokramer olmokramer referenced this pull request Oct 4, 2017
0 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.