Make `vw_pin_appeal` unique by `pin`, `year`, `case_no` #674

wrridgeway · 2024-12-09T15:41:58Z

default.vw_pin_appeal is not unique by pin, year, case_no as our docs suggest it should be. There are actual duplicates in iasworld.htpar that need to be removed. While I was trying to sort this out, Mirella told me we should also be much more parsimonious in terms of which values for htpar.heartyp we include in order to exclude things like PTAB adjustments and erroneous exemptions.

Yes, the only hearing types you should be pulling if you are strictly focusing on appeal data are the ones listed below:

O = Omitted Assessments, which are new for 2024 and by next year, there will the following new ones as well as others that will be used by the Tax side:

E = COE only

EE = Erroneous Exemptions (will likely change it to avoid conflict with E)

P = PTAB Refunds (Treasurer)

S = Specific Objections (Treasurer)

(After some initial confusion on my part, I confirmed she meant "only include 'A' and 'C'")

with new as (select year, count(*) as new from z_dev_wridgeway_default.vw_pin_appeal group by year),
old as (select year, count(*) as old from default.vw_pin_appeal group by year)

select new.year, new.new, old.old, new.new - old.old as diff from new
left join old on new.year = old.year

year	new	old	diff
2024	405164	435781	-30617
2023	243427	273843	-30416
2022	302331	302820	-489
2021	373640	373991	-351
2020	238032	238034	-2
2019	389870	391241	-1371
2018	531406	534183	-2777
2017	370217	373983	-3766
2016	446840	451081	-4241
2015	522393	526499	-4106
2014	340849	345294	-4445
2013	412460	416041	-3581
2012	392624	392637	-13
2011	317394	323076	-5682
2010	353612	359586	-5974
2009	449028	454532	-5504
2008	315669	321696	-6027
2007	336394	342098	-5704
2006	355976	359159	-3183
2005	232970	237025	-4055
2004	288567	294421	-5854
2003	318327	320938	-2611
2002	232716	235919	-3203
2001	198419	200379	-1960
2000	230578	232583	-2005
1999	156619	158653	-2034

select pin, year, case_no
from z_ci_make_vw_pin_appeal_unique_default.vw_pin_appeal
group by pin, year, case_no
having(count(*) > 1)

yields zero rows.

wrridgeway · 2024-12-09T19:06:52Z

dbt/models/default/default.vw_pin_appeal.sql

-- reason codes.
+-- CTE so that we can join reason descriptions onto cleaned reason codes.
 WITH reasons AS (
-    SELECT
-        htpar.*,
+    SELECT DISTINCT
+        htpar.parid,
+        htpar.taxyr,
+        htpar.caseno,
+        htpar.user38,
+        htpar.user104,
+        htpar.resact,
+        htpar.hrstatus,
+        htpar.cpatty,
+        -- user125 isn't used in the view, but it's FilingID and we want to
+        -- include it to make sure we're only dropping true dupes using DISTINCT
+        htpar.user125,


Pulling in the smallest number of columns I can because of the DISTINCT

dbt/models/default/default.vw_pin_appeal.sql

wrridgeway · 2024-12-09T19:08:45Z

dbt/models/default/default.vw_pin_appeal.sql

-LEFT JOIN {{ source('iasworld', 'pardat') }} AS pardat
+-- This is an INNER JOIN since there are apparently parcels in htpar that are
+-- not in pardat. See 31051000511064, 2008.
+INNER JOIN {{ source('iasworld', 'pardat') }} AS pardat


For some godforsaken reason there are PINs in htpar that are not in pardat.

[Question, non-blocking] Why remove these PINs, when the only side effect of their inclusion would be some appeals with null classes? We could just document that artifact in the view? I guess I don't feel super confident that in these cases the mistake lies in extraneous appeals, rather than missing rows from pardat. But if you do, I'm happy to move forward with this.

My thought was that we usually depend on pardat or the parcel shapefile as our parcel universe. Once we start letting pins from outside of those two sources into views our universe is going to be inconsistent across tables, views, etc. Personally, I trust pardat more than htpar.

wrridgeway · 2024-12-09T20:49:46Z

dbt/models/default/default.vw_pin_appeal.sql

+    WHERE htpar.cur = 'Y'
+        AND htpar.deactivat IS NULL
+        AND htpar.caseno IS NOT NULL
+        AND htpar.heartyp IN ('A', 'C')


This conditional means we'll only get strictly defined appeals - I'm not entirely positive that's what we want, but it is what we suggest our view is. If we allow in other values for this column we need to address three appeals in 2022 that have both "E" and "A" heartyp for the same pin, case_no, year.

wrridgeway · 2024-12-09T23:09:51Z

dbt/models/default/schema/default.vw_pin_appeal.yml

@@ -59,7 +65,7 @@ unit_tests:
          - {parid: "123", taxyr: "2024", cur: "Y", deactivat: null, class: "2.1-1)A"}
      - input: source("iasworld", "htpar")
        rows:
-          - {parid: "123", taxyr: "2024", cur: "Y", deactivat: null, cpatty: "1", caseno: "1"}
+          - {parid: "123", taxyr: "2024", cur: "Y", deactivat: null, cpatty: "1", caseno: "1", heartyp: "A", user125: "A", user38: "CC", user104: "A", resact: "A", hrstatus: "A", user100: "A", user101: "A", user89: "A"}


@jeancochrane this works, but I'm not sure if I've included extraneous columns.

It should work with only heartyp, since that's the only new column that's being used in a table filter or join. The rest of the new columns here after heartyp should be unnecessary, since we're not testing their output and the default that dbt chooses (null) won't affect joins or filters. Does that work as expected?

jeancochrane

Nice work tracking down some tricky behavior here! A couple thoughts below, but nothing serious.

Before we merge, could we also update the data catalog docs for iasworld.htpar.heartyp to include the enumerated values that you explained in the body of the PR? I assume we also need to add descriptions for A and C:

O = Omitted Assessments, which are new for 2024 and by next year, there will the following new ones as well as others that will be used by the Tax side:

E = COE only

EE = Erroneous Exemptions (will likely change it to avoid conflict with E)

P = PTAB Refunds (Treasurer)

S = Specific Objections (Treasurer)

dbt/models/default/default.vw_pin_appeal.sql

jeancochrane · 2024-12-10T16:23:53Z

dbt/models/default/default.vw_pin_appeal.sql

-LEFT JOIN {{ source('iasworld', 'pardat') }} AS pardat
+-- This is an INNER JOIN since there are apparently parcels in htpar that are
+-- not in pardat. See 31051000511064, 2008.
+INNER JOIN {{ source('iasworld', 'pardat') }} AS pardat


[Question, non-blocking] Why remove these PINs, when the only side effect of their inclusion would be some appeals with null classes? We could just document that artifact in the view? I guess I don't feel super confident that in these cases the mistake lies in extraneous appeals, rather than missing rows from pardat. But if you do, I'm happy to move forward with this.

Co-authored-by: Jean Cochrane <jeancochrane@users.noreply.github.com>

wrridgeway added 2 commits December 9, 2024 15:41

Make appeals uniue by pin, year, caseno

87bc911

Docs

f7bf637

wrridgeway self-assigned this Dec 9, 2024

wrridgeway added 8 commits December 9, 2024 15:53

Add uniqeuness test

f58cbb0

Excluding all appeals before 2021

3ef8db7

Add 'A' heartyp

44b3afe

Correct heartyp values

85f91c1

Drop actual dupes

ce5d1ae

Update new min row count

2c30b1e

Remove odd htpar parcels

f4ab2a2

Update min row count

8c1775d

wrridgeway commented Dec 9, 2024

View reviewed changes

dbt/models/default/default.vw_pin_appeal.sql Show resolved Hide resolved

wrridgeway commented Dec 9, 2024

View reviewed changes

wrridgeway added 6 commits December 9, 2024 19:14

Test unit test adjustment

24cd07d

Undo bad change

6fce362

Add back all hearing types, address few remaing dupes

be255cd

Commenting

a5fe98d

Update docs

3c788bc

Try to fix unit test

10e5f88

wrridgeway commented Dec 9, 2024

View reviewed changes

yaml ordering

52008f6

wrridgeway changed the title ~~Make vw pin appeal unique by pin, year, case_no~~ Make vw_pin_appeal unique by pin, year, case_no Dec 9, 2024

wrridgeway marked this pull request as ready for review December 9, 2024 21:02

wrridgeway requested a review from a team as a code owner December 9, 2024 21:02

wrridgeway commented Dec 9, 2024

View reviewed changes

jeancochrane reviewed Dec 10, 2024

View reviewed changes

jeancochrane approved these changes Dec 10, 2024

View reviewed changes

wrridgeway and others added 2 commits December 10, 2024 11:47

Update dbt/models/default/default.vw_pin_appeal.sql

fa2644c

Co-authored-by: Jean Cochrane <jeancochrane@users.noreply.github.com>

Remove extraneous columns from unit test

95fbb5d

wrridgeway added 3 commits December 10, 2024 11:58

Add hearing types

879658e

Linting

6b4a017

Formatting

f200b79

wrridgeway merged commit 4788fee into master Dec 10, 2024
7 checks passed

wrridgeway deleted the make-vw_pin_appeal-unique branch December 10, 2024 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `vw_pin_appeal` unique by `pin`, `year`, `case_no` #674

Make `vw_pin_appeal` unique by `pin`, `year`, `case_no` #674

wrridgeway commented Dec 9, 2024 •

edited

Loading

wrridgeway Dec 9, 2024 •

edited

Loading

wrridgeway Dec 9, 2024 •

edited

Loading

jeancochrane Dec 10, 2024

wrridgeway Dec 10, 2024

wrridgeway Dec 9, 2024 •

edited

Loading

wrridgeway Dec 9, 2024

jeancochrane Dec 9, 2024 •

edited

Loading

jeancochrane left a comment •

edited

Loading

jeancochrane Dec 10, 2024

Make vw_pin_appeal unique by pin, year, case_no #674

Make vw_pin_appeal unique by pin, year, case_no #674

Conversation

wrridgeway commented Dec 9, 2024 • edited Loading

wrridgeway Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

wrridgeway Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

jeancochrane Dec 10, 2024

Choose a reason for hiding this comment

wrridgeway Dec 10, 2024

Choose a reason for hiding this comment

wrridgeway Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

wrridgeway Dec 9, 2024

Choose a reason for hiding this comment

jeancochrane Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

jeancochrane left a comment • edited Loading

Choose a reason for hiding this comment

jeancochrane Dec 10, 2024

Choose a reason for hiding this comment

Make `vw_pin_appeal` unique by `pin`, `year`, `case_no` #674

Make `vw_pin_appeal` unique by `pin`, `year`, `case_no` #674

wrridgeway commented Dec 9, 2024 •

edited

Loading

wrridgeway Dec 9, 2024 •

edited

Loading

wrridgeway Dec 9, 2024 •

edited

Loading

wrridgeway Dec 9, 2024 •

edited

Loading

jeancochrane Dec 9, 2024 •

edited

Loading

jeancochrane left a comment •

edited

Loading