Enrollment Metrics Rework #214

johnbaldwin · 2020-05-13T05:20:41Z

Added new figures.pipeline.enrollment_metrics module to conditionally retrieve progress data from the platform depending on the states of StudentModule and LearnerCourseGradeMetrics models for the given CourseEnrollment

Running CourseGradeFactory().read(..) is costly and needed to retrieve dynamic grades when persistent grades is not enabled or supported (as is the case with Ginkgo). This PR addresses this

The existing get_average_progress method is renamed to get_average_progress_deprecated instead of just deleting it. This is so we have the code available in production to support troubleshooting and validation

The individual commits, commit messages and inline documentation should explain details for this PR

This module processes data to provide course enrollment metrics. Course enrollment is a unique learner/course pair. See the source code in this commit for documentation

- Added `course_enrollments_for_course` - Added `student_modules_for_course_enrollment` This is to support the course progress pipeline performance improvement code restructuring. The purpose of adding these functions is to move forward with using `figures.sites` as the source of platform data. To note, there are a number of places in the code that do not follow this. As we rework Figures, we will refactor code to use `figures.sites` instead of directly querying the platform models. Note the function name change from the existing functions. We are moving away from using the `get_` prefix for the site functions.

…work - Updated figures.pipeline.course_daily_metrics loader to use the new enrollment pipeline code - Renamed `get_average_progress` to `get_average_progress_deprecated` and retained if needed to help debug the new enrollment metrics - Updated tests to support these changes

johnbaldwin · 2020-05-13T05:40:52Z

I need to look into this. Pylint seems to think that the queryset is a list

https://travis-ci.org/github/appsembler/figures/jobs/686429867#L665

The code it complains about is this:

    most_recent_lcgm = LearnerCourseGradeMetrics.objects.filter(
        user=course_enrollment.user,
        course_id=str(course_enrollment.course_id)).order_by('date_for').last()

OmarIthawi

Thanks @johnbaldwin. I need to take another look at it. It's not a tiny code, so I could use some hints from you in the Logic section.

OmarIthawi · 2020-05-13T14:11:01Z

figures/pipeline/enrollment_metrics.py

+# Logic
+
+```
+for each learner+course


Great section! Would you mind adding references in the logic to which function I should take a look at e.g.

Suggested change

for each learner+course

for each learner+course # see `main` function

Will do!
Edit: After trying this out, I decided to be sparse with referencing which functions get called and focused on readability

OmarIthawi · 2020-05-13T14:11:14Z

figures/pipeline/enrollment_metrics.py

+    get newest lgcm record
+    get newest sm record
+
+    if not lcgm and not sm


Suggested change

if not lcgm and not sm

if not lcgm and not sm # see `func_123`

melvinsoft

@johnbaldwin I didn't have the time to review all the test, but I'm approving you since the logic changes makes total sense, and it's well documented.

Thanks! Great work!

tkeemon

I'm not in a position to test this locally, but the logic here looks good. Hopefully this speeds up the nightly grade calculation.

… (no-member)" See: pylint-dev/pylint-django#165 Also updated documentation for enrollment metrics

johnbaldwin · 2020-05-17T00:16:25Z

Fixed the pylint failure. Looks like an issue with pylint-django: pylint-dev/pylint-django#165

New code: figures.pipeline.enrollment_metrics

2ad0f40

This module processes data to provide course enrollment metrics. Course enrollment is a unique learner/course pair. See the source code in this commit for documentation

johnbaldwin requested review from melvinsoft, OmarIthawi and tkeemon May 13, 2020 05:20

johnbaldwin added 2 commits May 13, 2020 01:25

johnbaldwin force-pushed the john/rework-course-progress branch from 60ff839 to 3d82184 Compare May 13, 2020 05:26

OmarIthawi reviewed May 13, 2020

View reviewed changes

OmarIthawi self-requested a review May 13, 2020 14:12

melvinsoft approved these changes May 13, 2020

View reviewed changes

tkeemon approved these changes May 14, 2020

View reviewed changes

johnbaldwin added 2 commits May 16, 2020 16:04

Merge branch 'master' into john/rework-course-progress

df554ca

Addresses Pylint failure "Instance of 'list' has no 'order_by' member…

d40dc78

… (no-member)" See: pylint-dev/pylint-django#165 Also updated documentation for enrollment metrics

johnbaldwin force-pushed the john/rework-course-progress branch from 975a341 to d40dc78 Compare May 16, 2020 23:35

johnbaldwin merged commit 7961f4f into master May 17, 2020

johnbaldwin deleted the john/rework-course-progress branch May 17, 2020 00:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enrollment Metrics Rework #214

Enrollment Metrics Rework #214

johnbaldwin commented May 13, 2020

johnbaldwin commented May 13, 2020

OmarIthawi left a comment

OmarIthawi May 13, 2020

johnbaldwin May 13, 2020 •

edited

Loading

OmarIthawi May 13, 2020

melvinsoft left a comment

tkeemon left a comment

johnbaldwin commented May 17, 2020

	for each learner+course
	for each learner+course # see `main` function

	if not lcgm and not sm
	if not lcgm and not sm # see `func_123`

Enrollment Metrics Rework #214

Enrollment Metrics Rework #214

Conversation

johnbaldwin commented May 13, 2020

johnbaldwin commented May 13, 2020

OmarIthawi left a comment

Choose a reason for hiding this comment

OmarIthawi May 13, 2020

Choose a reason for hiding this comment

johnbaldwin May 13, 2020 • edited Loading

Choose a reason for hiding this comment

OmarIthawi May 13, 2020

Choose a reason for hiding this comment

melvinsoft left a comment

Choose a reason for hiding this comment

tkeemon left a comment

Choose a reason for hiding this comment

johnbaldwin commented May 17, 2020

johnbaldwin May 13, 2020 •

edited

Loading