Add custom message for 212s, 218s, and 219s#155
Conversation
hugo/layouts/report.html
Outdated
| </p> | ||
| </div> | ||
| {{ end }} | ||
|
|
There was a problem hiding this comment.
Is this the right location for this message?
There was a problem hiding this comment.
[Thought, non-blocking] Hmm, that's a good question. I do think the top of the page is at risk of drowning in alerts:
One way to think of this text might be as an alternate formulation of the desk review note that we show at the bottom of the page, tweaked for specific cases where we know desk review is very likely:
I'm curious what @ccao-jardine thinks!
jeancochrane
left a comment
There was a problem hiding this comment.
The overall structure here looks good! I think we just need to figure out if this is the right spot for this warning, or if we need to move it somewhere else.
Is it possible that there wouldn't be a 219 in south tri?
That does indeed seem to be the case, per default.vw_pin_universe:
select triad_name, count(*)
from default.vw_pin_universe
where class = '219'
and year = '2026'
group by 1| triad_name | count |
|---|---|
| City | 11 |
hugo/layouts/report.html
Outdated
| </p> | ||
| </div> | ||
| {{ end }} | ||
|
|
There was a problem hiding this comment.
[Thought, non-blocking] Hmm, that's a good question. I do think the top of the page is at risk of drowning in alerts:
One way to think of this text might be as an alternate formulation of the desk review note that we show at the bottom of the page, tweaked for specific cases where we know desk review is very likely:
I'm curious what @ccao-jardine thinks!
|
Agreed, we're racking up the alerts here. (Think there's a 212 multicard out there with a same-property sale in its top 5, that could rack up even more alerts?) I am open to design feedback. I did propose that it's best at the top of the page rather than the bottom. As a user, I'd rather know before seeing the characteristics and sales that this is a special property, rather than reading through all these characteristics and sales and then learning that they might not offer much by way of explanation to the AV. But I am just one user. Thoughts on adding it as extra text to the first disclaimer? Transparency is hard. |
That seems reasonable! This is how I'm imagining we'd format a consolidated first disclaimer, let me know if you had something else in mind:
I would also suggest we tweak the language slightly for the sake of clarity (mostly just simplifying and switching to active voice), though I leave the final decision about that up to Nicole:
|
Works for me. @ccao-jardine let us know what you think about Jean's suggested tweak and I can go ahead and make whatever edits we decide on |
|
I like the location and the updated language -- thanks! |
jeancochrane
left a comment
There was a problem hiding this comment.
Looking great! Mostly just pending the switch to PIN-level class instead of card-level class at this point.
| analyst_review_classes = {"212", "218", "219"} | ||
| analyst_review = str(df_target_pin["char_class"].iloc[0]) in analyst_review_classes |
There was a problem hiding this comment.
[Suggestion, required] Preserving some discussion that we had over chat yesterday: It's not ideal that we're pulling this class from the first card in the PIN, given that cards can have different classes, and those classes don't necessarily match up with the PIN-level class. We're going to try switching to PIN-level class here, which will require updating the upstream view in data-architecture to include a pin_class column in pinval.assessment_card.
Co-authored-by: Jean Cochrane <jeancochrane@users.noreply.github.com>
| HOMEVAL_ASSESSMENT_CARD_TABLE = "pinval.assessment_card" | ||
| HOMEVAL_ASSESSMENT_CARD_TABLE = ( | ||
| "z_ci_add_pin_class_to_custom_homeval_message_pinval.assessment_card" | ||
| ) |
There was a problem hiding this comment.
need to switch this back
jeancochrane
left a comment
There was a problem hiding this comment.
Good to go, pending the switch back to the prod table reference once ccao-data/data-architecture#1000 lands!
Co-authored-by: Jean Cochrane <jeancochrane@users.noreply.github.com>
Closes #154
Examples on staging:
Is it possible that there wouldn't be a 219 in south tri?