Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update desk review workbook for 2024 model #23

Merged
merged 7 commits into from
Feb 15, 2024

Conversation

jeancochrane
Copy link
Contributor

@jeancochrane jeancochrane commented Feb 7, 2024

This PR updates the export stage and the desk review template to work with the 2024 condo model. In the process, we also update the iasWorld export to make sure the format matches the new specification for this year.

This PR represents the condo equivalent of ccao-data/model-res-avm#199, ccao-data/model-res-avm#206, ccao-data/model-res-avm#209, and ccao-data/model-res-avm#213.

@jeancochrane jeancochrane marked this pull request as draft February 7, 2024 18:03
Comment on lines -121 to -123
flag_land_gte_95_percentile, flag_bldg_gte_95_percentile,
flag_land_gte_95_percentile,
flag_land_value_capped, flag_prior_near_to_pred_unchanged,
flag_pred_initial_to_final_changed,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flag_bldg_gte_95_percentile and flag_pred_initial_to_final_changed appear not to be present anymore in assessment_pin, so I removed them from the workbook flags. They also didn't seem to be present in the workbook template anyway.

select(meta_pin10, building_coord)

pin_sheet_name <- "PIN Detail"
bldg_sheet_name <- "Buildings"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sheet used to be called Building (PIN10), but unfortunately as I discovered when updating the res model, sheet names can't have spaces if we want to link to them via between-sheet hyperlink formulas.

@@ -341,45 +397,25 @@ upload_data_prepped <- assessment_pin %>%
select(meta_year, meta_pin, meta_card_num, meta_lline_num),
by = c("meta_year", "meta_pin")
) %>%
mutate(meta_pin10 = str_sub(meta_pin, 1, 10)) %>%
group_by(meta_pin10, meta_tieback_proration_rate) %>%
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By my reading, the group_by statements are no longer necessary in this query, since we don't need to report pred_pin10_final_fmv_bldg; but I could be wrong!

@jeancochrane jeancochrane marked this pull request as ready for review February 7, 2024 18:26
@jeancochrane jeancochrane marked this pull request as draft February 8, 2024 23:04
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard to see in the diff, but I made the following changes to this template:

  • Added column groups with colors and borders to both sheets to match the res workbook
  • Added two columns for non-arm's-length sale flag types
  • Tweaked the width of the Address and Municipality columns to better fit the data
  • Added Land Val. >= 95th Percentile column to the Flags section of the PIN Detail sheet, since this attribute was already present in the assessment_pin query but not being output to the sheet correctly
  • Moved Condo Unit S. F. column from the Flags group to the Characteristics group (this seems like it was previously a mistake, since the underlying variable is char_unit_sf and doesn't include the flag_ prefix like the rest of the flags do)
  • Recreated the final tweaks from Add final tweaks to desk review workbook based on Valuations feedback model-res-avm#213 in this pipeline

Comment on lines +187 to +190
# Handle a rare edge case where neighborhoods span multiple townships
# and so introduce duplicate pin10s; more details here:
# https://github.com/ccao-data/data-architecture/issues/275
distinct(meta_pin10) %>%
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it's OK to leave this bandaid in here for now, or do we need to spend some time resolving the root problem?

@@ -244,6 +341,13 @@ for (town in unique(assessment_pin_prepped$township_code)) {
writeFormula(
wb, pin_sheet_name,
assessment_pin_filtered$meta_pin,
startCol = 1,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to specify startCol in these formulas now, since there are two of them; otherwise, the second formula clobbers the first one (which is confusing API behavior, but it's what we have to work with).

@jeancochrane jeancochrane marked this pull request as ready for review February 9, 2024 21:53
The previous iteration of the note was not quite correct, implying that the flag had to do with the 95th percentile of land _value_ instead of land _square footage_.
@wrridgeway wrridgeway merged commit 841801a into master Feb 15, 2024
4 checks passed
@wrridgeway wrridgeway deleted the jeancochrane/condo-model-desk-review branch February 15, 2024 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants