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

Remove year from desk review join #97

Open
wants to merge 1 commit into
base: 2025-assessment-year
Choose a base branch
from

Conversation

Damonamajor
Copy link
Contributor

@Damonamajor Damonamajor commented Jan 31, 2025

When looking at #80, data from the final columns were returning as NA. This appears to be the issue of a join between condo runs where a previous iteration was done in the same year. Thus, the join could be on pin and year. Removing the year from the join results in valid outputs, and the dimensions of the join remain the same. However, I don't know if year was intended to be a part of the join for another reason.

@Damonamajor Damonamajor changed the title Remove year Remove year from desk review join Jan 31, 2025
Copy link
Contributor

@jeancochrane jeancochrane left a comment

Choose a reason for hiding this comment

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

Looks like we introduced the assessment_pin_old join in #41 to give some guidance to Valuations on new sales that we had missed in a prior "final" model run. I just checked in with @dfsnow and he says those columns aren't necessary in the desk review sheets anymore. How about we take this opportunity to remove them and the now-unnecessary assessment_pin_old code? We could either do that here, or fold the changes into #80 to minimize conflicts regarding changes to the Excel template file.

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.

Investigate why columns are missing from desk review spread sheets
2 participants