Bugfix/attendance calendars from bell schedules#88
Draft
sleblanc23 wants to merge 3 commits intomainfrom
Draft
Conversation
Contributor
ejoranlienea
left a comment
ejoranlienea
left a comment
There was a problem hiding this comment.
Would anything change about this object if the following joins were removed?
- course_section
- class_period
- bell
It looks like in each of these cases, the subsequent joins use the same keys that are already present and nothing else from these objects is used directly, so we may be able to trim this down by a few joins without affecting anything.
rlittle08
reviewed
Nov 30, 2023
| inner join bell_dates | ||
| on bell.k_bell_schedule = bell_dates.k_bell_schedule | ||
| -- limit to the duration of the section enrollment | ||
| where bell_dates.calendar_date between student_section_enr.begin_date and student_section_enr.end_date |
Collaborator
There was a problem hiding this comment.
I think we need to handle nulls in end_date here
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description & motivation
Addresses an edge case in the
fct_student_daily_attendancemodel. Students who only attend school part-time (only on Tuesday/Thursday, for example) are currently shown to be in attendance on days they don't go to school. This PR fixes that by constructing the calendar of possible attendance days for each student based on their class schedules.Changes to existing files:
fct_student_daily_attendance: Replace the calendar-constructing steps with a join to the newly added build model.New files created:
bld_ef3__student_school_days: Constructs the student-school level instructional calendars, including logic that was previously infct_student_daily_attendancefor removing non-instructional days and any that fall outside the student's enrollment. Put this in a separate model to keep it readable given of the large number of joins required.Tests and QC done:
Confirmed that it fixed the identified edge cases without creating any new inaccuracies for three schools in Jeffco. Additional QC to be done before marking ready for review.
Future ToDos & Questions:
It's possible that this creates more problems than it solves when bell schedule data is bad or incomplete.
PR Merge Priority: