-
Notifications
You must be signed in to change notification settings - Fork 16
Add popsafe-fips to geomapper
#1787
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
Conversation
Doing so makes it easier to add new mappings by reducing the number of lines with hard-coded geo/key names.
|
The provided county groupings don't seem to meet the stated requirements. Using our population data from 2020, there are many ungrouped counties and county groups with populations less than 20k (data file generated with script). Some counties and county groups are just barely below the cutoff and would likely meet the 20k threshold using updated population data, but there are many counties that are far (>50%) below the population requirement.
|
|
I'm checking the population results with Mina and will hold my final review until that gets resolved. Thank you for doing the initial analysis! |
|
The data provider wants to keep the county groups as-is. We should rename the new level to make it clear that it isn't generally applicable nor "population safe". Maybe |
dshemetov
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.
Looks good. Thanks for adding good tests too! Just one (non-blocking) question: does CHNG have any prop signals? Might be good to add some population methods if so.
| elif contained_geocode_type == "county" and container_geocode_type == "state": | ||
| crosswalk = self._crosswalks["fips"]["state"] | ||
| return set(crosswalk[crosswalk["state_id"] == container_geocode]["fips"]) | ||
| elif (contained_geocode_type in ("county", "fips", "popsafe-fips") and |
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.
praise: thanks for enforcing consistency!
|
@nmdefries
(use dashes so it's easier to parse out of the CSV filenames) |
krivard
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.
Modulo the name change, looks great!
| "pop": int, | ||
| "weight": float | ||
| "weight": float, | ||
| **{geo: str for geo in self._geos - set("nation")} |
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.
praise: nicely crystallized!
|
Will go with |
CHNG doesn't currently report any prop signals or directly use any population data, so we don't need to do this. |
krivard
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.
(best remove all implied relationships between chng groups and population)
Co-authored-by: Katie Mazaitis <[email protected]>
|
@krivard I won't get to splitting the |
krivard
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.
CHNG probably needs another couple weeks to fix their end, but approving this provisionally anyhow
We can't rely on the component counties of a group being listed individually, so reconstruct those fields ourselves based on the concatenated `fips_list` field. Split by the separator (pipe `|`) and save each result to a new column.
|
Splitting the |
…geomapper/chng-fips-map-generation
Co-authored-by: Dmitry Shemetov <[email protected]>
|
@korlaxxalrok This is ready to merge. |
…-map-generation generate local county mapping from CHNG spreadsheet
Description
Create new level in the geomapper to support changes to
chngdata. Background and design considerations.To allow this, add a
popsafe-fipslevel to the geomapper that codes high-population (>20k) counties with their official FIPS codes and low-population counties with a code of the format<two digit state FIPS>g<two digit group number within the state>, e.g. "01g05". Add mappings forfips->popsafe-fipsandpopsafe-fips->state.Changelog
data_proc/geomap/lowpop_county_groups.csvgeo_data_proc.py- functions to generatepopsafe-fipsmap tables from input county groups filefips->popsafe-fipsandpopsafe-fips->stategeomap.pytest_geomap.py