Feature/fct school acquisition metrics #52
Merged
Conversation
This file contains 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
updated descritpion on PR front page
allison-code-dot-org
approved these changes
Jan 16, 2024
dbt/models/marts/metrics/fct_weekly_school_acquisition_metrics.sql
Outdated
Show resolved
Hide resolved
bakerfranke
requested review from
jordan-springer
and removed request for
jordan-springer
jordan-springer
requested changes
Jan 19, 2024
| select | ||
| school_year, | ||
| sssy.school_year, | ||
| school_level_simple, |
There was a problem hiding this comment.
best practice here is to attach an alias to each column when joining (eg. sssy.school_year)
| where status like 'active %' | ||
| group by 1,2,3,4,5 | ||
| group by 1,2,3,4,5,6,7 |
There was a problem hiding this comment.
{{ dbt_utils.group_by(7) }}
… feature/fct_school_acquisition_metrics
… feature/fct_school_acquisition_metrics
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR is a second pass at defining a weekly school acquisition metrics table for use with MIR.
int_school_weeks
A major contribution to this PR is the definition of a new intermediate table called
int_school_weeks. The attempt here is to solve the problem of how to align school year weeks with standard ISO weeks and this table proposes a solution. (This issue is mentioned in this ticket)Please read the comments in the code, and the methodology of
int_school_weekscarefully because it will define how we define weeks of the school year moving forward. Its design is an attempt to mimicint_school_yearsin terms of how analysts or modelers would join to it, and also to not conflict with what have become our standard school year boundaries.I think that is a good solution moving forward, and better than what we currently use in terms of standardizing school weeks. It will mean validation might get tricky because some schools might shift from one schoolyard to another based on how we currently do reporting, but I think those differences well in the end be marginal and it's worth this change to standardize the way we segment data by weeks of the school year.
**THIS PR IS LARGELY ABOUT APPROVING
int_school_weeksand merging it into main.The weekly school acquisition metrics code, was updated to incorporate this new table, but follow up work is to validate.
Followup work