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

Feature: Showing Only Applied Parts Of Handgrading Rubric #667

Merged
merged 5 commits into from
Oct 21, 2022
Merged

Feature: Showing Only Applied Parts Of Handgrading Rubric #667

merged 5 commits into from
Oct 21, 2022

Conversation

developStorm
Copy link
Contributor

@developStorm developStorm commented Oct 19, 2022

@developStorm
Copy link
Contributor Author

developStorm commented Oct 19, 2022

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

I only applied "visible" annotation/checkbox, and this is how it looks like on student's end

image

@james-perretta
Copy link
Contributor

james-perretta commented Oct 19, 2022 via email

@developStorm
Copy link
Contributor Author

developStorm commented Oct 20, 2022

Thanks for clarifying! I reviewed Gradescope's practice and looks like they are not hiding checkboxes:
image

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
@developStorm developStorm marked this pull request as ready for review October 20, 2022 04:13
@developStorm
Copy link
Contributor Author

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
eecs-autograder/ag-client-typescript#147

I've roughly tested it locally and it works without problems. Let me know what you think 😺

@james-perretta
Copy link
Contributor

@developStorm I don't suppose you could add a few test cases for this?

@james-perretta
Copy link
Contributor

james-perretta commented Oct 20, 2022

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"?

@developStorm
Copy link
Contributor Author

developStorm commented Oct 20, 2022

@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.

@james-perretta
Copy link
Contributor

james-perretta commented Oct 21, 2022

@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). In the interest of time so that you can release grades ASAP, I'm going to make a local copy of your branch, make the fixes and run the tests, and then make a new PR to merge.

Edit: I'm going to merge this as-is as soon as the tests finish running and let future me fix the pycodestyle warnings.

@james-perretta
Copy link
Contributor

Tests passing locally, merging with minor pycodestyle warnings.

@james-perretta james-perretta merged commit 23b2c18 into eecs-autograder:develop Oct 21, 2022
@developStorm
Copy link
Contributor Author

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
Copy link
Contributor

@james-perretta Yes, you're right, sorry about that! I can add that along with getting CI up and running again...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to only show applied parts of handgrading rubric to students
2 participants