Added normalize column to export grade book to remote gradebook #190

amir-qayyum-khan · 2016-01-29T11:54:02Z

Background

This PR resolves #147, resolves #187 and resolves #188

What is done in this PR

Studio Updates: None
LMS Updates:

Added checkbox on legacy dashboard to indicate grades are normalize or not
On grade book export added column normalize with value 1 if gadebook is normalize else 0.

@pdpinch

Raw scores and max points are more useful to instructors in the remote gradebook than the calculated scores sent now. Fixes #36 @pdpinch

amir-qayyum-khan · 2016-01-29T12:06:03Z

@pwilkins I made changes can you verify it.
2nd If I rebase it the link to legacy dashboard will be lost. So what should I do to keep these changes in mitocw/master

amir-qayyum-khan · 2016-02-01T15:39:10Z

lms/djangoapps/instructor/views/legacy.py

+                course,
+                get_grades=True,
+                use_offline=use_offline,
+                get_score_max=True


for
normalise = true => get_score_max=False
normalise = False => get_score_max=ture

amir-qayyum-khan · 2016-02-02T14:12:17Z

@pwilkins it seems there was issue with grade calculation logic . I tried to fix that. Now
we have answers like below

For normalize = True

For normalize = False

Are these results ok with you?

cc @pdpinch

amir-qayyum-khan · 2016-02-03T11:33:15Z

@pdpinch @pwilkins

if a problem is gradeable and student has not attempted it yet then existing system mark it
graded_total = Score(0.0, 1.0, True, section_name, None)
https://github.com/edx/edx-platform/blob/master/lms/djangoapps/courseware/grades.py#L461
which means this section is not graded yet
If I change this logic and make it
graded_total = Score(0.0, max points, True, section_name, None)
Then the issue appears here is that if student has not attempted the problem then his marks will be 0/max points which will look like he has attempt and he is fail.
Existing code they solved this issue by adding a dummy value

Do you want me to change this logic?

pdpinch · 2016-02-03T11:46:06Z

I'm not sure I understand. Are you saying that 0.0/1.0 points indicates the user hasn't attempted the problem and 0.0/max points would make it look like the user attempted & got 0 points?

Wouldn't a problem with a max of 1.0 look the same, whether the user attempted or not?

amir-qayyum-khan · 2016-02-03T12:02:16Z

I'm not sure I understand. Are you saying that 0.0/1.0 points indicates the user hasn't attempted the
problem and 0.0/max points would make it look like the user attempted & got 0 points?

yes. This is what i am saying

Wouldn't a problem with a max of 1.0 look the same, whether the user attempted or not?

For non-normalize grades the grade sheet appears like that 0/max point for students who attempted the problem and 0/1 for student which do not attempted it . This indicates student did not attempt.

but for normalize it will show attempted (fail) and not attempted = 0/1

PR you told me to create so that you can discuss with ormsbee.
#196

amir-qayyum-khan · 2016-02-04T16:53:49Z

lms/djangoapps/instructor/views/legacy.py

+                            if get_score_max is True:
+                                add_grade(grade_item['label'], earned, possible=possible)
+                            else:
+                                add_grade(grade_item['label'], grade_item['percent'], possible=1)


@pdpinch @pwilkins

Reverted code for normalize calculation as previous code, shared in meeting https://github.com/mitocw/edx-platform/blame/df5b2dadda636184116fa394105ea37e3ab3b867/lms/djangoapps/instructor/views/legacy.py#L731

Added possible=1 because code above (https://github.com/mitocw/edx-platform/pull/190/files#diff-5dd41ae768fed10fe37df53a1d9e78c8R268) is expecting a tuple (earned, possible).

If this is an issue the i need to modify code to release data without possible in
https://github.com/mitocw/edx-platform/pull/190/files#diff-5dd41ae768fed10fe37df53a1d9e78c8R268 ??

pdpinch · 2016-02-08T20:59:12Z

lms/djangoapps/instructor/views/legacy.py

+                    gradeset = student_grades(
+                         student, request, course, keep_raw_scores=get_raw_scores, use_offline=use_offline
+                    )
+                    log.debug(u'student=%s, gradeset=%s', student, gradeset)


Is there a reason why we use student_grades in this code branch progress_summary in the other code branch?

Can't we use progress_summary for both?

amir-qayyum-khan · 2016-02-08T21:41:25Z

Fixed @pdpinch
Grade book summery is show problems with actual name instead of tags like Ex 01 etc

pdpinch · 2016-02-09T15:43:29Z

lms/djangoapps/instructor/views/legacy.py

+                            percent = earned / float(possible)
+                            percent = round(percent * 100 + 0.05) / 100
+                            add_grade(label, percent, possible=1)
+


I think this can be more DRY, since the only difference between the branches is the calculation of the percentages and the call to add_grade()

pdpinch · 2016-02-09T15:45:21Z

It's going to be a problem if we export the long (user-friendly) name instead of the short assignment name & number. Those short labels appear on the student progress page in the graph, so that data has to be in the view somewhere.

pdpinch · 2016-02-09T15:47:05Z

I think you've got this merging into the wrong branch, hence the merge conflicts. Instead of master, merge it into mitx-cypress.1-hotfix.3 (but check with @bdero before actually merging!)

…ssue using gradebook summery

amir-qayyum-khan · 2016-02-10T13:15:26Z

Done @pdpinch
@bdero which branch i should target for this fix. mitx-cypress.1-hotfix.3 is 2942 ahead from my branch?

pdpinch · 2016-02-10T13:59:33Z

lms/djangoapps/instructor/views/legacy.py

+                                add_grade(grade_item['label'], earned, possible=possible)
+                            else:
+                                add_grade(grade_item['label'], grade_item['percent'], possible=1)
+                        except (IndexError, KeyError):


Can you add a comment explaining when you expect to hit this exception

actually it was @pwilkins work (try, catch). I havent changed it much. I am just loading max points from progress_summery.

@pwilkins can you explain me why you used this try catch statement?

cc @pdpinch

amir-qayyum-khan · 2016-02-11T09:51:08Z

closed by #201

Fix xss in date

Send raw score and max points to remote gradebook.

d924c80

Raw scores and max points are more useful to instructors in the remote gradebook than the calculated scores sent now. Fixes #36 @pdpinch

amir-qayyum-khan added the in progress label Jan 29, 2016

amir-qayyum-khan self-assigned this Jan 29, 2016

amir-qayyum-khan added CCX legacy dashboard labels Jan 29, 2016

amir-qayyum-khan added Needs Review and removed in progress labels Jan 29, 2016

amir-qayyum-khan reviewed Feb 1, 2016
View reviewed changes

amir-qayyum-khan mentioned this pull request Feb 3, 2016

Fixed calculation of max_pts for problems which are not attempted yet #196

Closed

amir-qayyum-khan reviewed Feb 4, 2016
View reviewed changes

amir-qayyum-khan mentioned this pull request Feb 4, 2016

WIP Fixed calculation of max_pts for problems which are not attempted yet openedx/edx-platform#11425

Closed

pdpinch reviewed Feb 8, 2016
View reviewed changes

amir-qayyum-khan force-pushed the enhancement/aq/add_normalization_info_lagacy_dashboard_remote_gradebook_mitocw#147 branch from 71fd151 to 4328dbb Compare February 8, 2016 21:39

pdpinch reviewed Feb 9, 2016
View reviewed changes

amir-qayyum-khan force-pushed the enhancement/aq/add_normalization_info_lagacy_dashboard_remote_gradebook_mitocw#147 branch from 4328dbb to 395478e Compare February 10, 2016 13:06

Added normalize column and checkbox in UI and fixed grade max point i…

6a9278f

…ssue using gradebook summery

amir-qayyum-khan force-pushed the enhancement/aq/add_normalization_info_lagacy_dashboard_remote_gradebook_mitocw#147 branch from 395478e to 6a9278f Compare February 10, 2016 13:11

pdpinch reviewed Feb 10, 2016
View reviewed changes

amir-qayyum-khan closed this Feb 11, 2016

amir-qayyum-khan removed the Needs Review label Feb 11, 2016

emZubair pushed a commit that referenced this pull request Sep 30, 2020

Merge pull request #190 from edx/security-fix/fix-xss-date

314fae8

Fix xss in date

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added normalize column to export grade book to remote gradebook #190

Added normalize column to export grade book to remote gradebook #190

amir-qayyum-khan commented Jan 29, 2016

amir-qayyum-khan commented Jan 29, 2016

amir-qayyum-khan Feb 1, 2016

amir-qayyum-khan commented Feb 2, 2016

amir-qayyum-khan commented Feb 3, 2016

pdpinch commented Feb 3, 2016

amir-qayyum-khan commented Feb 3, 2016

amir-qayyum-khan Feb 4, 2016

pdpinch Feb 8, 2016

amir-qayyum-khan commented Feb 8, 2016

pdpinch Feb 9, 2016

pdpinch commented Feb 9, 2016

pdpinch commented Feb 9, 2016

amir-qayyum-khan commented Feb 10, 2016

pdpinch Feb 10, 2016

amir-qayyum-khan Feb 10, 2016

amir-qayyum-khan commented Feb 11, 2016

Added normalize column to export grade book to remote gradebook #190

Added normalize column to export grade book to remote gradebook #190

Conversation

amir-qayyum-khan commented Jan 29, 2016

Background

What is done in this PR

amir-qayyum-khan commented Jan 29, 2016

amir-qayyum-khan Feb 1, 2016

Choose a reason for hiding this comment

amir-qayyum-khan commented Feb 2, 2016

For normalize = True

For normalize = False

amir-qayyum-khan commented Feb 3, 2016

pdpinch commented Feb 3, 2016

amir-qayyum-khan commented Feb 3, 2016

amir-qayyum-khan Feb 4, 2016

Choose a reason for hiding this comment

pdpinch Feb 8, 2016

Choose a reason for hiding this comment

amir-qayyum-khan commented Feb 8, 2016

pdpinch Feb 9, 2016

Choose a reason for hiding this comment

pdpinch commented Feb 9, 2016

pdpinch commented Feb 9, 2016

amir-qayyum-khan commented Feb 10, 2016

pdpinch Feb 10, 2016

Choose a reason for hiding this comment

amir-qayyum-khan Feb 10, 2016

Choose a reason for hiding this comment

amir-qayyum-khan commented Feb 11, 2016