Refactor to use new simplified pinval.vw_assessment_card data model#85
Conversation
| {{ template "card-content" | ||
| (dict "card" $card | ||
| "var_labels" $.Params.var_labels | ||
| "Params" $.Params |
There was a problem hiding this comment.
While debugging an issue with .Params being unavailable in the context of the card-content template, I realized that we can just pass the whole object into the template in order to make it available. That way we don't need to add more arguments to the template in order to access stuff from the .Params object.
| # Currently we don't need more than one run because comps and cards are using the same | ||
| # final run ID, but we're keeping this map around with the expectation that we may | ||
| # need it for a more permanent fix | ||
| RUN_ID_MAP = {"2025-02-11-charming-eric": "2025-02-11-charming-eric"} |
There was a problem hiding this comment.
See here for an explanation of this (confusing) change, and why I'm not bothering cleaning it up just yet: ccao-data/data-architecture#856 (comment)
wagnerlmichael
left a comment
wagnerlmichael
left a comment
There was a problem hiding this comment.
Nice work. I took a look at the updated PIN on the staging site and it looks good with all the columns intact. I did have on question about the constants file that I left in my review
scripts/generate_pinval/constants.py
Outdated
| # It's helpful to factor these tables out into shared constants because we often | ||
| # need to switch to dev tables for testing | ||
| PINVAL_ASSESSMENT_CARD_TABLE = "z_dev_jecochr_pinval.vw_assessment_card" | ||
| PINVAL_COMP_TABLE = "z_dev_jecochr_pinval.vw_comp" |
There was a problem hiding this comment.
[Question, non-blocking]: So is the idea here to essentially use this a params file where the developer changes this locally to select which asset to pull from during development? So you'd change this to production tables before merging this into main? Do I have that right?
There was a problem hiding this comment.
Are you asking about the constants.py module in its entirety, or specifically about the PINVAL_*_TABLE constants? The PINVAL_*_TABLE constants are indeed intended to offer us a way of controlling which assets we use during development, but I think the constants.py module is useful for a wider range of tasks. For example, we probably won't change RUN_ID_MAP very often, and it should largely stay the same between dev and prod; however, if we ever do need to change the mapping between chars run ID and comps run ID, the fact that it is factored out to a centralized constant will make that change simpler.
There was a problem hiding this comment.
Also, good catch on changing the PINVAL_*_TABLE constants back prior to merging! I reverted the change in 22e0ca9 so they should point to the prod tables now.
This reverts commit dc1b791.
jeancochrane
deleted the
jeancochrane/855-avoid-selecting-star-from-modelassessment_card-in-pinvalvw_assessment_card
branch
2 participants
This PR updates our Hugo report generation code to handle the changes to the
pinval.vw_assessment_carddata model that we're making in ccao-data/data-architecture#856.In addition, we fold in one small change to improve the experience of testing changes to the data model: Factoring out constants for
PINVAL_ASSESSMENT_CARD_TABLEandPINVAL_COMP_TABLE. By centralizing the references to these two tables, we can more easily change them when we need to test a corresponding change indata-architecture.See PIN
06171140020000on the staging site for an example of a deployed report.Connects ccao-data/data-architecture#855.