Update QC report tags, code, and docs to split tri town close reports from non-tri towns #574
Conversation
… from non-tri towns
| python3 scripts/export_models.py --selector select_qc_report_town_close_tri --where "taxyr = '$TAXYR' and township_code = '$TOWNSHIP_CODE'" | ||
| ``` | ||
|
|
||
| The script will output the reports to the `dbt/export/output/` directory, and will print the | ||
| names of the reports that it exports during execution. | ||
| Use the `select_qc_report_town_close_non_tri` selector to output reports for | ||
| a **non-tri town**: | ||
|
|
||
| ``` | ||
| python3 scripts/export_models.py --selector select_qc_report_town_close_non_tri --where "taxyr = '$TAXYR' and township_code = '$TOWNSHIP_CODE'" | ||
| ``` |
There was a problem hiding this comment.
This new selector interface feels slightly clunky to me: The selector names are quite long, and I feel like it would be more intuitive if the export_models script could just infer whether the township is a tri town or a non-tri town based on the taxyr and township code. However, making that change would require either tighter coupling between export_models and the town close reports, or (more likely) an additional script like export_town_close_report on top of export_models that could implement that coupling. It might make sense for us to move in that direction in the future, but for now I went with the most incremental possible change that would satisfy the requirement of splitting tri town close reports from non-tri town reports.
There was a problem hiding this comment.
I think that makes sense! Can you add an issue linking to this comment so we don't forget about it? Could be a good jr project IMO.
There was a problem hiding this comment.
Good call! See #576.
dbt/README.md
Outdated
| In general, we tag the models that comprise our town close QC reports using the | ||
| `qc_report_town_close` tag. Models with this tag will be included in all town | ||
| close QC reports. If a model should be included in town close reports for tri | ||
| towns but not for non-tri towns, we tag it instead with | ||
| `qc_report_town_close_tri`. Likewise, the `qc_report_town_close_non_tri` tag | ||
| marks models that should be included in town close reports for _non-tri_ towns. | ||
| In both cases, we filter the models for a specific township code (like "70") | ||
| and tax year during export. |
There was a problem hiding this comment.
suggestion (non-blocking): Could be helpful here to define (or link out to a definition for) tri-town vs non-tri town. It's a pretty jargony term.
There was a problem hiding this comment.
That makes sense, I fleshed out this paragraph and added a link to the Townships doc in 2e6848c.
| python3 scripts/export_models.py --selector select_qc_report_town_close_tri --where "taxyr = '$TAXYR' and township_code = '$TOWNSHIP_CODE'" | ||
| ``` | ||
|
|
||
| The script will output the reports to the `dbt/export/output/` directory, and will print the | ||
| names of the reports that it exports during execution. | ||
| Use the `select_qc_report_town_close_non_tri` selector to output reports for | ||
| a **non-tri town**: | ||
|
|
||
| ``` | ||
| python3 scripts/export_models.py --selector select_qc_report_town_close_non_tri --where "taxyr = '$TAXYR' and township_code = '$TOWNSHIP_CODE'" | ||
| ``` |
There was a problem hiding this comment.
I think that makes sense! Can you add an issue linking to this comment so we don't forget about it? Could be a good jr project IMO.
| @@ -891,23 +891,23 @@ models: | |||
| description: '{{ doc("view_vw_report_town_close_prior_year_card_code_5s") }}' | |||
| config: | |||
| tags: | |||
| - qc_report_town_close | |||
| - qc_report_town_close_non_tri | |||
There was a problem hiding this comment.
Perfectly good tagging schema already off the rails.
jeancochrane
deleted the
jeancochrane/mark-some-qc-town-close-reports-as-non-tri
branch
I learned from Will today that a few of the town close QC reports are specific to non-tri towns, meaning that he doesn't generate them for tri towns. This PR tweaks our tags, code, and docs for town close QC reporting to reflect this fact and allow us to export either tri town close QC reports or non-tri town close QC reports.
I tested this change by running the two versions of town close reports and confirming that the script exported the correct reports for each kind. To export a tri town:
To export a non-tri town: