[1/3] Migrate Will's town close QC reports to dbt models #554

jeancochrane · 2024-07-25T16:14:33Z

Overview

This PR is the first of three following up on #534 by adding a bunch of models and Excel templates to recreate the QC reports that Will sends out on town close.

This PR migrates the following reports that all share common logic:

0 land value
0 value
Vacant class, bldg value
Improved class, no bldg value
Class does not equal LUC
500k increase, 1m decrease

The subsequent PRs finishing the migration are #556 and #557. I've tried to group PRs according to reports that share common logic, with the third PR being a grab bag of miscellaneous reports.

We make a few other small changes to support this effort:

Existing QC report views (qc.vw_change_in_ahsap_values and qc.vw_report_town_close_neg_asmt_value) get refactored to remove hardcoded dates, in line with other QC models
The dbt/scripts/export_models.py script is extended to support reading a config.meta.export_format.columns attribute from the schema config, which is used to set formatting options for the output Excel workbooks that can't be set in the template (such as right-aligning text data columns)

Testing

To recreate 2024 Jefferson township reports for the purposes of testing, run the following command in the dbt/ subdirectory:

python3 scripts/export_models.py --select tag:qc_report_town_close --where "township_code = '71' and taxyr = '2024'" --rebuild

The reports will be output to the dbt/export/output/ directory.

…proach model

…oach view

jeancochrane · 2024-07-25T16:15:37Z

.flake8

+[flake8]
+ignore=
+    # Whitespace before ':', incompatible with black
+    E203,
+    # Line break before binary operator, incompatible with black
+    W503


At some point should probably just switch this project over to ruff, but in the meantime this flake8 config change is necessary to fix some conflicts between black and flake8.

Whoops I think I just biffed this with #564. Will probably need a run through with ruff.

Good catch! Ruff seems to make this config unnecessary so I removed it in 0e41f3e.

jeancochrane · 2024-07-25T16:17:22Z

dbt/models/qc/docs.md

@@ -117,19 +138,213 @@ In contrast to `qc.vw_neg_asmt_value`, this view directly performs filtering
 for negative values.
 {% enddocs %}

-# vw_sale_mydec_null_values
+# vw_report_town_close_0_land_value


Note that the docs blocks for the new reports are listed in the order that Will runs them, which made it easier for me to track my progress during development, but I can rearrange them to be in alphabetical order if we have a strong preference for that.

Let's just quickly order them alphanumerically yeah, for consistency with the other docs.

Makes sense, done in 3744e48! Remind me whether we normally also order the schema files this way?

Schema files should be alphanumerically ordered. The docs files are kind of a mixed bag (some are broken up by section), but are usually alphanumeric also.

Sounds good, I also reordered the schema file in 10e033f. Sorry for the annoying diff! I didn't change anything and checked with a dbt compile that the graph still compiles correctly, which should hopefully rule out any obvious copy/paste errors.

dbt/models/qc/qc.vw_change_in_ahsap_values.sql

dbt/models/qc/qc.vw_iasworld_asmt_all_with_prior_year_values.sql

dbt/models/qc/qc.vw_report_town_close_neg_asmt_value.sql

dbt/models/qc/schema.yml

…red asmt view

dbt/models/qc/docs.md

jeancochrane · 2024-07-26T22:02:41Z

dbt/models/qc/schema.yml

+          - name: '% Change'
+            index: M
+            horizontal_align: right


Per the PR body, this is the new syntax for specifying formatting that should be applied to the export workbook at runtime. This is necessary because (as far as I can tell) you can't set horizontal alignment in an Excel template before you populate it with data using openpyxl.

todo: When you build up the docs for the QC reporting exports, let's be sure to enumerate the possible options here.

Good flag, I've added a note on this to #539.

dbt/scripts/export_models.py

dbt/models/qc/qc.vw_iasworld_asmt_all_with_prior_year_values.sql

dfsnow · 2024-08-02T17:21:49Z

.flake8

+[flake8]
+ignore=
+    # Whitespace before ':', incompatible with black
+    E203,
+    # Line break before binary operator, incompatible with black
+    W503


Whoops I think I just biffed this with #564. Will probably need a run through with ruff.

dfsnow · 2024-08-02T17:32:52Z

dbt/models/qc/docs.md

@@ -117,19 +138,213 @@ In contrast to `qc.vw_neg_asmt_value`, this view directly performs filtering
 for negative values.
 {% enddocs %}

-# vw_sale_mydec_null_values
+# vw_report_town_close_0_land_value


Let's just quickly order them alphanumerically yeah, for consistency with the other docs.

dbt/export/templates/assessed_and_market_values.xlsx

dbt/models/qc/qc.vw_iasworld_asmt_all_with_prior_year_values.sql

dbt/models/qc/qc.vw_report_town_close_500k_increase_1m_decrease.sql

dbt/models/qc/qc.vw_report_town_close_improved_class_without_bldg_value.sql

dfsnow · 2024-08-02T17:58:27Z

dbt/models/qc/schema.yml

+          - name: '% Change'
+            index: M
+            horizontal_align: right


todo: When you build up the docs for the QC reporting exports, let's be sure to enumerate the possible options here.

dbt/scripts/export_models.py

…when result set has only 1 row

dfsnow

Nice, this looks great @jeancochrane!

jeancochrane and others added 23 commits July 17, 2024 17:18

Migrate 0 land value QC report

b8674b3

Sketch out town close QC queries based on asmt prior year values

42f6950

Migrate "Res parcels not set to cost approach" QC report

f47872d

Migrate "OVRRCNLDs discrepancy checks" QC report

7e16513

Move qc_vw_incorrect_val_method_revcode_eq_1 data test to new cost ap…

f694913

…proach model

Migrate "Bldg parcel class mismatch" QC report

d234235

Add docs to QC report town close models, with TODO Inquire query numbers

7f2a4cf

Migrate "289s" QC report

74edc74

Migrate "500k increase, 1m decrease" QC report

100993c

Migrate "Res multicodes" QC report

589f46a

Migrate "Card code 5s possibly not at 100%" QC report for comdat

97a0899

Migrate all "Card code 5s possibly not at 100%" QC reports

2d36d11

Factor out vw_iasworld_sales_latest_sale_since_2021 QC view

e9c3e88

Migrate "Res edit" QC report with some TODOs

af7f3a5

Add export templates and complete migration of QC reports

1d5cd26

Fix "Possibly not at 100%" QC reports

663dede

Fix formatting of "Possibly not at 100%" percent change columns

493063a

Fix "500k increase, 1m decrease" QC report formatting

bb3403c

Fix "Bldg parcel class mismatch" QC report formatting

b13206b

Fix "OVRRCNLD discrepancy checks" QC report formatting

4de1200

Fix "Res multicodes" QC report formatting

10f2b61

Fix "Res parcels not set to Cost Approach" formatting

6ec6bd6

Fix "Res edit" QC report formatting

36d5a10

jeancochrane linked an issue Jul 25, 2024 that may be closed by this pull request

Automate Will's QC reports with dbt models and a simple script #525

Closed

jeancochrane added 4 commits July 25, 2024 17:21

Document QC report models ahead of review

32a7ea2

Add docs for intermediate QC models

3ed53a2

Remove unused vw_incorrect_val_method docs

93792f1

More reader-friendly description for res_parcels_not_set_to_cost_appr…

0251a70

…oach view

jeancochrane commented Jul 25, 2024

View reviewed changes

Refactor qc.vw_report_town_close_500k_increase_1m_decrease to use sha…

7d22a9d

…red asmt view

jeancochrane added 2 commits July 26, 2024 21:39

Rollback QC report models that will be added in third PR

5e4bc7b

Rollback QC report models that will be added in second PR

5b86b18

jeancochrane changed the title ~~Migrate Will's town close QC reports to dbt models~~ [Do not merge] Migrate Will's town close QC reports to dbt models Jul 26, 2024

jeancochrane changed the title ~~[Do not merge] Migrate Will's town close QC reports to dbt models~~ [1/3] Migrate Will's town close QC reports to dbt models Jul 26, 2024

This was referenced Jul 26, 2024

[2/3] Migrate Will's town close QC reports to dbt models #556

Merged

[3/3] Migrate Will's town close QC reports to dbt models #557

Merged

jeancochrane commented Jul 26, 2024

View reviewed changes

jeancochrane requested a review from dfsnow July 26, 2024 22:09

Format parids as numbers in QC reports to avoid scientific notation

60026b8

jeancochrane marked this pull request as ready for review July 30, 2024 21:33

jeancochrane requested a review from a team as a code owner July 30, 2024 21:33

jeancochrane commented Jul 30, 2024

View reviewed changes

dbt/models/qc/qc.vw_iasworld_asmt_all_with_prior_year_values.sql Outdated Show resolved Hide resolved

Filter out leasehold properties from 0 land value QC report

1f04f90

dfsnow reviewed Aug 2, 2024

View reviewed changes

jeancochrane added 4 commits August 5, 2024 17:01

Merge branch 'master' into jeancochrane/migrate-more-will-qc-reports

a4c4d4a

Remove unnecessary flake8 config now that we're using ruff

0e41f3e

Only convert QC reports to data tables if the result set is non-empty

ac37578

Reorder qc schema docs alphanumerically

3744e48

jeancochrane force-pushed the jeancochrane/migrate-more-will-qc-reports branch from 47810e9 to 3744e48 Compare August 5, 2024 19:59

jeancochrane added 6 commits August 5, 2024 20:12

Append reason code descriptions to aprval.reascd in QC reports

49363b7

Fix edge case bug in export_models.py script that can raise an error …

4faba2f

…when result set has only 1 row

Document card code 5s query in 500k_increase_1m_decrease QC report

2ebab55

Format PINs as explicitly 14-digit numbers in export_models.py script

3af51f0

Remove stray docs block artifact from qc/docs.md

0b3fc92

Reorder qc/schema.yml alphanumerically

10e033f

jeancochrane requested a review from dfsnow August 6, 2024 20:10

dfsnow approved these changes Aug 6, 2024

View reviewed changes

jeancochrane merged commit 26b998e into master Aug 7, 2024
9 checks passed

jeancochrane deleted the jeancochrane/migrate-more-will-qc-reports branch August 7, 2024 14:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1/3] Migrate Will's town close QC reports to dbt models #554

[1/3] Migrate Will's town close QC reports to dbt models #554

jeancochrane commented Jul 25, 2024 •

edited

Loading

jeancochrane Jul 25, 2024

dfsnow Aug 2, 2024

jeancochrane Aug 5, 2024

jeancochrane Jul 25, 2024

dfsnow Aug 2, 2024

jeancochrane Aug 5, 2024

dfsnow Aug 6, 2024

jeancochrane Aug 6, 2024

jeancochrane Jul 26, 2024

dfsnow Aug 2, 2024

jeancochrane Aug 5, 2024

dfsnow Aug 2, 2024

dfsnow Aug 2, 2024

dfsnow Aug 2, 2024

dfsnow left a comment

[1/3] Migrate Will's town close QC reports to dbt models #554

[1/3] Migrate Will's town close QC reports to dbt models #554

Conversation

jeancochrane commented Jul 25, 2024 • edited Loading

Overview

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dfsnow left a comment

Choose a reason for hiding this comment

jeancochrane commented Jul 25, 2024 •

edited

Loading