-
Notifications
You must be signed in to change notification settings - Fork 8
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
Feature: Showing Only Applied Parts Of Handgrading Rubric #667
Conversation
Ah, the reason I mentioned performance in the issue description was because
I was conflating this with things that have to be computed for all groups'
final submissions at once, but obviously it only happens to one at a time,
so performance isn't an issue.
What you have looks good. There is still the consideration of whether it
makes sense to hide checkboxes that give positive points--I think that
warrants a discussion but wouldn't majorly change the implementation.
Thanks for the PR!
…On Wednesday, October 19, 2022, Raymond Nook ***@***.***> wrote:
This is a PoC for this feature. I wanted to check if this looks good
before moving forward with implementing the instructor toggle option for
this.
[image: image]
<https://user-images.githubusercontent.com/59678453/196818573-78ca4db0-d925-4956-9671-c45bc84539e2.png>
[image: image]
<https://user-images.githubusercontent.com/59678453/196818507-36d4ea79-5586-49ca-8df5-6e024a02575b.png>
—
Reply to this email directly, view it on GitHub
<#667 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA7IXMEAWL23SV7KWODNJA3WEB3EJANCNFSM6AAAAAARJSJCJ4>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Thanks for clarifying! I reviewed Gradescope's practice and looks like they are not hiding checkboxes: However I'm pretty sure what @ekaprits wants is to hide every unapplied rubric from student, including those with positive points ; ) so I'll probably just do that first Or making this behavior optional is also possible. I can see if that can be done without adding too much complexity. |
and corresponding model change
Alright! I've implemented both frontend and backend part for this feature. It involves this PR and these two PRs: eecs-autograder/ag-website-vue#500 I've roughly tested it locally and it works without problems. Let me know what you think 😺 |
@developStorm I don't suppose you could add a few test cases for this? |
And following up on the checkbox hiding, I'm hesitant to have the default behavior be hiding unselected checkboxes regardless of the points strategy selected. Can you double check that that's truly the behavior that you need, rather than that only being the case for "start at max and subtract"? |
@james-perretta Thanks for the review, and tests are ready now! I double-confirmed with the professor, he intends to use this feature to minimize the rubric leakage (i.e. to prevent students share rubrics with other students from subsequent semesters). From this perspective, he believes hiding all unapplied rubrics is necessary. This is different from Gradescope's design - which is more about preventing students from the misconception that they are being docked points. I added a tooltip in settings page to emphasis the effect of this feature. |
@developStorm Ok thanks, we can go with this for the time being. I'll update the issue with a note to expand on the implementation to allow for fine-grained control of which annotations and checkboxes to hide. I see a couple of minor linter errors (I have yet to finish migrating my CI to github actions since Travis CI changed their free tier, sorry about that). Edit: I'm going to merge this as-is as soon as the tests finish running and let future me fix the pycodestyle warnings. |
Tests passing locally, merging with minor pycodestyle warnings. |
Thanks @james-perretta! I didn't notice there's a linter in-place so I formatted my code with black's default rules. Maybe we should consider adding linter info to the local env setup guide? |
@james-perretta Yes, you're right, sorry about that! I can add that along with getting CI up and running again... |
Resolves eecs-autograder/autograder.io#40