ccao-data / data-architecture

Codebase for CCAO data infrastructure construction and management
https://ccao-data.github.io/data-architecture/
6 stars 4 forks source link

Changes to dbt tests following review from other teams #308

Closed jeancochrane closed 7 months ago

jeancochrane commented 8 months ago

Some changes that we need to make to our dbt tests:

jeancochrane commented 7 months ago

A few questions that came up for me while working through these edits:

  • Update class_mismatch tests so that we're only looking at res (i.e. no entry in comdat)

This seems pretty straightforward, just moving the *_matches_pardat_class tests to a QC view joined against iasworld.comdat and excluding any PINs that have a match in comdat, is that right?

  • Change matching logic so that we're looking for at least one dweldat class that matches pardat

I think we already do this due to the fact that row_values_match_after_join looks for any matches:

https://github.com/ccao-data/data-architecture/blob/bdbf827ad6a3d63248aeb2bd559570f35e625ad0/dbt/models/iasworld/schema/iasworld.dweldat.yml#L1127-L1128 https://github.com/ccao-data/data-architecture/blob/bdbf827ad6a3d63248aeb2bd559570f35e625ad0/dbt/tests/generic/test_row_values_match_after_join.sql#L38-L39

  • Add a test to ensure there are no minor classes on PINs where there's a major class component

I don't really get what this means, perhaps because I don't understand the distinction between a major class and a minor class (or maybe I took bad notes). Isn't the major class just the first number (e.g. 2-) and the minor class is the subsequent numbers (e.g. -11)?

  • Update land major class tests to accommodate EE/EX classes

It seems like we are maybe already doing this, no? The code in question:

https://github.com/ccao-data/data-architecture/blob/ad14e3b7cfcf47411a7a28ee269d268c1db49a1c/dbt/models/iasworld/schema/iasworld.land.yml#L316-L320

  • Add test to ensure that prorated res PINs with cards have at least one of the cards in both PINs associated with the tieback

I think this has something to do with checking pardat.tieback when pardat.tiebldgpct IS NOT NULL AND pardat.ofcard IS NOT NULL, but I'm not sure how to interpret "have at least one of the cards in both PINs associated with the tieback" (probably my fault for not taking better notes).

  • Update address tests to check against legdat instead of owndat

I'm not quite sure what to do here since our well-formed address tests check owndat.addr1, which seems to be a concatenation of address fields and doesn't have a direct match to any field in legdat.

  • Add test to ensure entries in legdat with missing addresses have at least a 0 in address number or municipality

I assume this is referring to legdat.adrno and legdat.cityname. My guess as to the specific query we want is something like "check that adrno = 0 when adrstr IS NULL, but I'm not totally sure how cityname (or just municipality, if cityname isn't the right field) fits in here.

  • Add test to ensure legdat street directionals are correct

I think we may be testing this already:

https://github.com/ccao-data/data-architecture/blob/b58f85a74e1b4aa02a05e11c4703dedbb21ed1f7/dbt/models/iasworld/schema/iasworld.legdat.yml#L27-L30

Notes from call

Some notes from our discussion on these issues: