Repository navigation
Rename repo, remove deprecated Quarto doc, and update README - #141
Merged
Merged
Conversation
jeancochrane
changed the title
jeancochrane
marked this pull request as ready for review
September 30, 2025 20:44
wagnerlmichael
approved these changes
Oct 2, 2025
wagnerlmichael
left a comment
wagnerlmichael
left a comment
Member
There was a problem hiding this comment.
This looks good to me, thanks for doing the testing. Somewhat relatedly, I command F'd pinval in data architecture and found some refs, but it seems like we might be okay with leaving those, especially the dbt assets. Some are comments/docs about the app, although doesn't seem like high priority either way.
Workflow testing once merged makes sense to me as well.
jeancochrane
added a commit
that referenced
this pull request
Dec 9, 2025
… Positron config files (#152) This PR updates the HomeVal report generation script to match the changes we made in ccao-data/data-architecture#936, which tweaked the materialization for the HomeVal data tables in our Athena data lake, and in the process renamed the tables to remove the `vw_` prefix that we use to mark Athena views. Luckily, we factored out the table names to a centralized constants module in #141, so changing the `generate_homeval.py` script is very simple. In addition updating the constant variables, we only need to tweak one comment and the analytics dashboard script, which is a separate R project and so doesn't use the constants file. The only tricky part of the change is that we have to alter one small part of the `generate_homeval.py` code where we previously assumed that all columns starting with `shap_*` represented numeric SHAP values, an assumption that breaks down now that we include `shap_run_id` as a column in the `pinval.assessment_card` table. Since I was messing around with the Positron/VSCode [Python debugger](https://code.visualstudio.com/docs/python/debugging) while debugging the `shap_run_id` issue, I also took a minute to fold in a quick tweak to the `.gitignore` file for the project to ignore all local VSCode config files. This was relevant because I created a [`launch.json` config file](https://code.visualstudio.com/docs/debugtest/debugging-configuration#_launch-configurations) in order to debug the `generate_homeval.py` script execution. See here for a `generate-homeval` workflow run that deploys a sample PIN (01011000040000) to the staging site: https://github.com/ccao-data/homeval/actions/runs/20070650417 I also reran the analytics dashboard locally and it worked as expected. Evidence that these are the only blocks of code across all of our repos that reference the HomeVal Athena models: https://github.com/search?q=org%3Accao-data%20pinval.vw_&type=code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
This PR makes two main changes to the codebase:
pinval➡️homevalto match the recent renaming of the repoWe also take a quick pass at the README to make sure the development instructions are up-to-date, and add a new section on deployment.
Testing
I tested this locally on a random PIN to confirm the script continues to work:
Note that we are not able to test on CI, since the
generate-homevalworkflow will not be available in the Actions tab of this repo until we merge this PR to the main branch. I'll plan to do a quick test on prod after merging and will put up a fast follow PR to fix any bugs that shake out in the process. This is not an ideal deployment workflow, but since we're not actively developing or deploying this app right now, I figure that the stakes are not very high.Migration
A few changes to note once you pull these changes in:
hugo/content/pinval-reports/subdirectory will not be automatically moved tohugo/content/homeval-reports/, so if you want to preserve those reports you should rename the subdirectory yourselfscripts/generate_pinval/.venv/, you will have to manually move it toscripts/generate_homeval/