feature/:Enhanced dim_course_section to expose section_characteristics descriptors

[smckee-tnedu]

Description & motivation

After the fix for course_level_characteristics, the visibility of section characteristics was gone. This set of changes brings that visibility back.

Breaking changes introduced by this PR:

This change requires a new xwalk: xwalk_section_characteristics.csv. An example looks like this:

section_characteristic_descriptor,indicator_name
Fall Block,is_fall_block
Spring Block,is_spring_block
Year-long,is_year_long

PR Merge Priority:

• Low
• Medium
• High

Changes to existing files:

• models/core_warehouse/dim_course_section.sql : Uses a new build model to expose the descriptors that come in on the sections_characteristics field.

New files created:

• models/build/edfi_3/courses/bld_ef3__section__wide_section_characteristics.sql : Builds a set of wide columns that can be accordioned in dim_course_section.

Tests and QC done:

This has been tested within stadium_tennessee.

edu_wh PR Review Checklist:

Make sure the following have been completed before approving this PR:

• Description of changes has been added to Unreleased section of CHANGELOG.md. Add under ## New Features for features, etc.
• Code has been tested/checked for Databricks and Snowflake compatibility - EA engineers see Databricks checklist here
• Reviewer confirms the grain of all tables are unchanged, OR any changes are expected, communicated, and this PR is flagged as a breaking change (not for patch release)
• If a new configuration xwalk was added:
  • The code is written such that the xwalk is optional (preferred), and this behavior was tested, OR
  • The code is written such that the xwalk is required, and the required xwalk is added to edu_project_template, and this PR is flagged as breaking change (not for patch release)
  • A description for the new xwalk has been added to EDU documentation site here
• If a new configuration variable was added:
  • The code is written such that the variable is optional (preferred), and this behavior was tested, OR
  • The code is written such that the variable is required, and a default value was added to edu_project_template, and this PR is flagged as breaking change (not for patch release)
  • A description for the new variable has been added to EDU documentation site here

[AngelicaLastra]

@smckee-tnedu this is a really clean PR! Thank you for catching this gap in the dim_course_section model!

There are a few small things to clean up before merge, but they are very small:

• In bld_ef3__section__wide_section_characteristics:

  • left outer join should be left join to match every other model in the repo.
  • I am wondering if we need the outer=True? what is the benefit of pushing through rows without a section characteristic?

• In dim_course_section:

  • The comment -- todo: add characteristic indicators on line 80 is stale, we should remove it so it doesn't cause confusion in the future.

On the YML side, three things need to be added:

• bld_ef3__section__wide_section_characteristics is not registered in _edfi_3__courses.yml. It needs an entry with config: tags: ['core'] and a column entry for section_characteristics_array to match the pattern established by bld_ef3__course_char__combined_wide.
• In dim_course_section.yml, section_characteristics_array is missing a column entry (the course_level_characteristics_array entry is a good template to follow). The "Important Business Rules" block in the model description also only references xwalk_course_level_characteristics and the course-level is_* columns; it should be updated to mention xwalk_section_characteristics and the section-level indicator columns as well.

These are all very minor, final touch up changes. The code runs successfully, does what it intends to do, and does not negatively affect grain or other aspects of the model. Thank you again for this work!

[AngelicaLastra]

Since we are adding them now through this PR, we no longer need the stale todo. We can remove this comment to ensure we don't cause confusion in the future.

[smckee-tnedu]

Yep.

[AngelicaLastra]

Very small, picky thing, can we use left join instead? It's the standard for the repo even if it means the same thing.

[smckee-tnedu]

You can, yes. They mean the same but I've always used the word "outer" just because in my history in IT (I'm old) I've always trained junior developers and explicitly saying "outer" has been helpful in reducing confusion.

[AngelicaLastra]

I wonder if we need the outer=true here? What would be the benefit of creating rows for course sections without characteristics for this model?

[smckee-tnedu]

I don't think it's needed. I'ts been awhile since I wrote this but I might have been thinking that I saw outer=true as sort of a standard you guys were doing for these things? So that maybe if you had to troubleshoot data in the table, you'd have all the rows to look at and not just the rows that contained useful data? I don't know, to be honest.

[rlittle08]

marking for 0.7.0 because it requires xwalk update. very likely to release in ~August