-
Notifications
You must be signed in to change notification settings - Fork 15
Update report params #430
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 report params #430
Conversation
jeancochrane
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! I have some questions about why the dvc.lock file previously had some weird values in it, but I assume that was caused by us not fully updating the lockfile last year. Leaving the questions below in case you have further insight.
| - meta_class | ||
| - meta_card_num | ||
| - meta_sale_document_num | ||
| model.seed: 2024 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Question, non-blocking] Why do you think we were previously pinned to the prior year for this seed and for the ratio study years below? Did we just forget to update them last year?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This didn't change from modeling in 2024 to modeling in 2025, and i believe it should have. that params.yaml entry that was removed but still persisting in dvc.lock combined with this makes me think we forgot to push the final lockfile (or that there was none).
| year: '2024' | ||
| date: '2024-01-01' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Question, non-blocking] I'm also curious why these values were set to 2023/2024, when I would expect them to have been 2024/2025 for the North tri reassessment year.
| - shp_parcel_mrr_side_ratio | ||
| - shp_parcel_num_vertices | ||
| pv: | ||
| multicard_yoy_cap: 2.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Question, non-blocking] What's up with this attribute disappearing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed here
Updating: