Skip to content

Add hhs and nation to DV #935

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

Merged
merged 2 commits into from
Mar 19, 2021
Merged

Add hhs and nation to DV #935

merged 2 commits into from
Mar 19, 2021

Conversation

chinandrew
Copy link
Contributor

Description

Replaces #616.

Cherrypicked 588f805 and made one small typo change.

Changelog

Itemize code/test/documentation changes and files added/removed.

  • Added functions for mapping counties to new geos, following same pattern as other geos (yes, there's some code duplication. figure we can refactor separately.)

Fixes

@krivard
Copy link
Contributor

krivard commented Mar 17, 2021

too-many-branches, alas

@chinandrew chinandrew changed the title Add hhs and nation Add hhs and nation to DV Mar 17, 2021
@chinandrew
Copy link
Contributor Author

chinandrew commented Mar 17, 2021

too-many-branches, alas

:( converted if/else block it to a dict for now.

@krivard krivard requested a review from mariajahja March 18, 2021 14:13
@krivard
Copy link
Contributor

krivard commented Mar 18, 2021

@mariajahja if this is good we'll want to update the copy on the magic server once it's merged

Copy link
Member

@mariajahja mariajahja left a comment

Choose a reason for hiding this comment

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

Actual PR looks good and some quick checks for county/hrr/state line up with existing estimates.

Noticed that there were some params.json changes from the version running on the magic server, but there some small adjustments needed (dropped f-strings prefixes and change from 60->70 days of updates). @krivard OK for me to open a separate PR for these?

@krivard
Copy link
Contributor

krivard commented Mar 19, 2021

@mariajahja yep, separate PR.

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.

3 participants