ccao-data / model-res-avm

Automated valuation model for all class 200 residential properties in Cook County (except vacant land and condos)
GNU Affero General Public License v3.0
20 stars 3 forks source link

Flesh out characteristics in desk review workbooks #218

Closed jeancochrane closed 4 months ago

jeancochrane commented 4 months ago

This PR makes some more updates to the desk review workbooks in line with the feedback we got last week:

Closes https://github.com/ccao-data/model-res-avm/issues/217.

jeancochrane commented 4 months ago

@dfsnow Before I merge, would you be able to spot check the tweaks in https://github.com/ccao-data/model-res-avm/pull/218/commits/21806e444ba58068c3266145fd984c7446ab13ce to make sure I'm understanding the intersection between class and APTS/NCU correctly? My understanding is:

Based on this I made the following changes:

dfsnow commented 4 months ago

@dfsnow Before I merge, would you be able to spot check the tweaks in 21806e4 to make sure I'm understanding the intersection between class and APTS/NCU correctly? My understanding is:

  • meta_class is set at the PIN level, while char_class is set at the card level
  • We filter char_apts and char_ncu by char_class in the ingest stage for training and assessment data
  • char_ncu is set to 0 when char_class != "212" in the ingest stage

Based on this I made the following changes:

  • Use and display char_class instead of meta_class for cards and comps (which are defined on the card level)
  • Don't filter char_apts by class, but do call format_char_apts for all 3 data sets to convert to numerals
  • Continue setting char_ncu to NA for all 3 data sets when the class code is not 212 (to account for the fact that the ingest stage sets it to 0 in these cases)

    • Per the first bullet above, we're now doing this filtering on char_class instead of meta_class for the cards and comp cases

Yep, that perfectly tracks with my understanding. Let's merge it and get the new sheets!