Further refactoring for the geo coding utility #217

dshemetov · 2020-08-13T04:01:42Z

The work for this #215

dshemetov · 2020-08-28T04:37:04Z

I am done with this refactor!

A summary of the changes:

added documentation and consolidated geocode source file info in /data_proc/geomap/README.md
added the functions zip_to_state_code, zip_to_state_id (and the convert_* versions), zip_to_msa and convert_zip_to_msa
added two functions add_geocode and replace_geocode meant to consolidate the logic in the utility and reduce the code size by a factor of 5. These functions work along side with the rest of the deprecated functions and are meant to replace e.g. zip_to_msa(df, ...) with replace_geocode(df, "zip", "msa", ...).
added tests for all new functions
cleaned geo_data_proc.py, removed unnecessary functions, separated derived crosswalks (shortcut crosswalks) from source crosswalks, consolidated the JHU UID hand modifications here
renamed functions that referred to fips or county interchangeably to consistently use fips, e.g. zip_to_county to zip_to_fips
enforced the string type on all geocodes, with zero padding as necessary
renamed instances of stcode to state_code for clarity
removed non-JHU UID functions for JHU conversion

dshemetov · 2020-08-28T05:24:27Z

Admittedly, this is a lot of changes. And for what it's worth, I don't plan on making such big PRs in the future. I believe, however, that this code is correct, because:

the tests ensure the two new functions have the same output as James' previous functions, on the given test cases
we total the values across the test dataframe columns after geocoding and verify that the data sum is identical

There may still be glitches in specific codings:

the JHU UID to FIPS conversion has some new additions and may need small revisions to its handling of edge cases
the FIPS to HRR conversion uses different files, the weights have an average absolute difference of 0.017 (normalized to 1)

krivard · 2020-08-28T18:25:48Z

Verify that this zip-fips mapping is not substantively different from the 02_20_uszips.csv mapping claimed to be updated in 2020.

dshemetov · 2020-08-29T04:34:25Z

Some updates:

Lower cased the state abbreviations
Updated the ZIP/FIPS population table to use the updated data from here (based on the total population count, this data appears to be from 2018), which will help @jingjtang's work in Quidel
- Contacted the source about their ZIP/FIPS data, they said "...currently we are using the 2010 US Census mapping for ZCTAs to counties and are using our own GIS algorithm to calculate map point zips to counties (using recent county shape data)." It is still unclear how they have updated population values per region.
We will continue to use the US Census data for the ZIP/FIPS mapping, for source reliability @jsharpna
- For future reference, I looked into the differences in weights given by simplemaps and the US census. The average absolute weights difference between the file above and the 2010 Census information is ~1e-05; the correlation between the twocolumns is ~1 - 1e-09.

jsharpna · 2020-09-02T19:33:06Z

We need to come to a decision, here are the pros and cons of using the 02_20_uszip file for weights...
pros: possibly updated to be consistent with the 2018 populations
cons: not clear what the source data is, not easy to update when new census data is available

Since we don't know where these weights come from, we can at least try to verify that they are consistent with the marginal population totals. To do this we...

multiply the fips-zip weight from this file by the zip population - this should give us the fips x zip population
sum that grouped by fips then if this matches the fips population then good

dshemetov · 2020-09-02T20:31:08Z

My apologies, it appears I made a large error in comparing the weights between the two sources in the comment above. The ZIP/FIPS weights in the two sources are actually almost identical. I have updated the comment to reflect the correct information.

dshemetov · 2020-09-02T20:46:51Z

@jsharpna, I ran your suggested query. The difference between the ZIP population total and the ZIP transformed to FIPS population total is 113.45 (out of 326 million), using simplemaps weights, and 147.0 using Census weights directly.

jsharpna · 2020-09-03T00:19:17Z

Ok @krivard, given that @dshemetov seems to think that the weights are just from the 2010 CB then I propose

we use the same weight file that we currently are (derived from 2010 CB)
we use most recent CB populations at the fips and zip level
this is subtle, but I think that we should match the population base geo to the base geo for the signal - for example, jhu uses fips (ish) then the populations should be aggregated from the fips level. (this could be precomputed in geomapper - but is more complicated).

dshemetov · 2020-09-11T19:22:52Z

@jsharpna and I found population estimates at the county level from US Census Bureau. I will use the 2019 estimates.

dshemetov · 2020-09-17T23:08:13Z

It turns out that Puerto Rico is not in the 2019 estimates above. Population estimates for all of Puerto Rico in 2019 are available, but not at the municipality level. I think that if we want municipality-level prop signals, we should use the 2010 data for Puerto Rico in our FIPS population file. The alternative is not reporting population-adjusted signals for Puerto Rico. I am in the process of implementing this solution, but if someone thinks of something better, let me know.

krivard · 2020-09-18T14:30:52Z

@dshemetov go ahead with the 2010 population figures for Puerto Rico municipalities.

@jsharpna We'll want to call out the discrepancy in the API documentation, probably as a new Population section in https://github.com/cmu-delphi/delphi-epidata/blob/main/docs/api/covidcast_geography.md. You and Dmitry should coordinate to write that section next week.

* README * data processing script * demo/tutorial notebooks Changes include: * updated data sources * Added state->HHS region mappings * new HRR weights * new JHU UID mappings by hand * 2019 population from US Census Bureau * Puerto Rico population from 2010 * Shortcut crosswalks Removed unused code

* Removed old sources, since we now pull from the internet * Renamed cross files, since they are tables * Added new mapping tables * Updated population figures * Added HHS mapping

* added the functions zip_to_state_code, zip_to_state_id (and the convert_* versions), zip_to_msa and convert_zip_to_msa * added two functions add_geocode and replace_geocode meant to consolidate the logic in the utility and reduce the code size by a factor of 5. These functions work along side with the rest of the deprecated functions and are meant to replace e.g. zip_to_msa(df, ...) with replace_geocode(df, "zip", "msa", ...). * renamed functions that referred to fips or county interchangeably to consistently use fips, e.g. zip_to_county to zip_to_fips * enforced the string type on all geocodes, with zero padding as necessary * renamed instances of stcode to state_code for clarity * removed non-JHU UID functions for JHU conversion * updated tests to match Bugfixes: * Removed .0000, 9xxx in output mappings - fixes most of #254 * Puerto Rico deaths should now be reported - fixes #179 * Generally fixes #215

chinandrew · 2020-09-20T02:14:20Z

_delphi_utils_python/delphi_utils/geomap.py

-                               fips_col: str = 'fips',
-                               stcode_col: str = 'st_code'):
-        """create st_code column from fips column
+        assert (


I'd use a ValueError here since it's a user input that's problematic.

chinandrew · 2020-09-20T02:18:08Z

_delphi_utils_python/delphi_utils/geomap.py

-                                 state_id_col='state_id',
-                                 full=False):
-        """create state_id column from county (fips) column
+### DEPRECATED FUNCTIONS BELOW


Should we add DeprecationWarnings to all of these when run?

chinandrew · 2020-09-20T02:24:14Z

I have tests failing, one of which may be due to pandas sorting. If so, adding sort=True to the pandas .merge should solve.

----------- coverage: platform linux, python 3.6.9-final-0 -----------
Name                                                                                        Stmts   Miss  Cover   Missing
-------------------------------------------------------------------------------------------------------------------------
/home/andrew/Documents/covidcast-indicators/_delphi_utils_python/delphi_utils/__init__.py       7      0   100%
/home/andrew/Documents/covidcast-indicators/_delphi_utils_python/delphi_utils/archive.py      153      0   100%
/home/andrew/Documents/covidcast-indicators/_delphi_utils_python/delphi_utils/export.py        11      0   100%
/home/andrew/Documents/covidcast-indicators/_delphi_utils_python/delphi_utils/geomap.py       296     18    94%   151, 283, 421, 502, 693, 737, 788, 854, 898, 907, 934, 938, 946, 970, 998, 1003, 1029, 1060
/home/andrew/Documents/covidcast-indicators/_delphi_utils_python/delphi_utils/utils.py          8      0   100%
-------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                         475     18    96%

======================================================================================================== short test summary info ========================================================================================================
FAILED test_geomap.py::TestGeoMapper::test_add_population_column - assert 172060 == 263790
FAILED test_geomap.py::TestGeoMapper::test_add_code - ValueError: operands could not be broadcast together with shapes (4,2) (3,2)

On linting, I get a number of errors, some of are in the deprecated functions and we could ignore.

************* Module delphi_utils.geomap
delphi_utils/geomap.py:3:5: C0303: Trailing whitespace (trailing-whitespace)
delphi_utils/geomap.py:13:61: C0303: Trailing whitespace (trailing-whitespace)
delphi_utils/geomap.py:413:0: C0301: Line too long (110/100) (line-too-long)
delphi_utils/geomap.py:492:0: C0301: Line too long (103/100) (line-too-long)
delphi_utils/geomap.py:546:0: C0301: Line too long (103/100) (line-too-long)
delphi_utils/geomap.py:844:0: C0301: Line too long (103/100) (line-too-long)
delphi_utils/geomap.py:887:0: C0301: Line too long (103/100) (line-too-long)
delphi_utils/geomap.py:961:0: C0301: Line too long (104/100) (line-too-long)
delphi_utils/geomap.py:1018:0: C0301: Line too long (104/100) (line-too-long)
delphi_utils/geomap.py:1080:0: C0301: Line too long (104/100) (line-too-long)
delphi_utils/geomap.py:1113:0: C0301: Line too long (104/100) (line-too-long)
delphi_utils/geomap.py:1:0: C0302: Too many lines in module (1137/1000) (too-many-lines)
delphi_utils/geomap.py:1137:0: C0305: Trailing newlines (trailing-newlines)
delphi_utils/geomap.py:90:4: W0102: Dangerous default value CROSSWALK_FILEPATHS (builtins.dict) as argument (dangerous-default-value)
delphi_utils/geomap.py:343:8: R1705: Unnecessary "else" after "return" (no-else-return)
delphi_utils/geomap.py:391:4: R0201: Method could be a function (no-self-use)
delphi_utils/geomap.py:442:4: R0201: Method could be a function (no-self-use)
delphi_utils/geomap.py:624:22: E1136: Value 'state_table' is unsubscriptable (unsubscriptable-object)
delphi_utils/geomap.py:686:35: W0613: Unused argument 'msa_col' (unused-argument)
delphi_utils/geomap.py:49:0: R0904: Too many public methods (30/20) (too-many-public-methods)

Also annoying docstring style nitpicks, courtesy of pydocstyle. Removed the warnings for deprecated functions

delphi_utils/geomap.py:50 in public class `GeoMapper`:
        D400: First line should end with a period (not 'i')
delphi_utils/geomap.py:91 in public method `__init__`:
        D205: 1 blank line required between summary line and description (found 0)
delphi_utils/geomap.py:91 in public method `__init__`:
        D400: First line should end with a period (not 's')
delphi_utils/geomap.py:110 in public method `load_crosswalk`:
        D401: First line should be in imperative mood (perhaps 'Load', not 'Loads')
delphi_utils/geomap.py:192 in public method `convert_fips_to_mega`:
        D400: First line should end with a period (not 'g')
delphi_utils/geomap.py:192 in public method `convert_fips_to_mega`:
        D403: First word of the first line should be properly capitalized ('Convert', not 'convert')
delphi_utils/geomap.py:208 in public method `megacounty_creation`:
        D202: No blank lines allowed after function docstring (found 1)
delphi_utils/geomap.py:208 in public method `megacounty_creation`:
        D400: First line should end with a period (not 'n')
delphi_utils/geomap.py:208 in public method `megacounty_creation`:
        D403: First word of the first line should be properly capitalized ('Create', not 'create')
delphi_utils/geomap.py:357 in public method `add_population_column`:
        D401: First line should be in imperative mood (perhaps 'Append', not 'Appends')
delphi_utils/geomap.py:402 in public method `fips_to_megacounty`:
        D400: First line should end with a period (not ')')
delphi_utils/geomap.py:402 in public method `fips_to_megacounty`:
        D403: First word of the first line should be properly capitalized ('Convert', not 'convert')

_delphi_utils_python/data_proc/geomap/geo_data_proc.py

_delphi_utils_python/delphi_utils/geomap.py

chinandrew · 2020-09-20T06:11:07Z

_delphi_utils_python/delphi_utils/geomap.py

+        }
+
+    # Utility functions
+    def load_crosswalk(self, from_code, to_code):


If this is just used by other methods and not public facing, may want to make this protected or private

_delphi_utils_python/delphi_utils/geomap.py

chinandrew · 2020-09-20T06:24:34Z

_delphi_utils_python/delphi_utils/geomap.py


-        Return:
+        Return
+        ---------
            data: copy of dataframe


Add more description here

chinandrew · 2020-09-30T21:33:04Z

@chinandrew I think those are the long URL lines. Is there are a preferred way to handle those?

if its a URL it's probably fine to leave it. might be able to do backslashes but that might actually make things less readable. Some of thse look like normal dosctrings though

dshemetov · 2020-09-30T21:38:44Z

Oh oops, thanks for catching those. Just fixed those up.

dshemetov · 2020-09-30T21:51:39Z

Just realized there are some renamings in this refactor that need to be smoothed out with the previous extension of the utility to JHU. Should be just naming fixes.

_delphi_utils_python/delphi_utils/geomap.py

* Correct README information about JHU fixes * Remove JHU NYC county splitting * Add Dmitry to authorship comments in geo_data_proc and geomap

…icators into rf_geo_refactor

krivard · 2020-10-01T19:20:00Z

Merge is pending completion of corresponding documentation

* Change 'national' -> 'nation' for consistency with #207 * Enforce FIPS and ZIP from_code when going to 'nation' level

* if 'bucket_name' param is empty, do not archive * typo fix to comment in geo_data_proc

…icators into rf_geo_refactor

* keep the JHU entries for the counties that Kansas City contributes to (were deleted previously, leading to missing counts) * fix bug that deleted the Puerto Rico Unassigned and Out of State JHU UIDs * delete the New York County splitting that was previously necessary * add New York County UIDs back to the crosswalk

krivard

delphi_utils lints and tests
jhu lints (with python3.6; 3.7+ still has a pytest recursion problem) and tests
emr_hosp is a retired signal, is exempt from linting, but tests pass
verified no other signals are currently using GeoMapper

jhu/delphi_jhu/geo.py

dshemetov self-assigned this Aug 25, 2020

dshemetov marked this pull request as ready for review August 28, 2020 04:37

This was referenced Sep 8, 2020

JHU changed Puerto Rico death reporting, causing us to fail to report it #179

Closed

JHU generating bogus ".0000" geo id #254

Closed

krivard mentioned this pull request Sep 17, 2020

Consider adding COVIDcast support for HHS regions cmu-delphi/delphi-epidata#192

Closed

krivard added 3 commits September 18, 2020 14:16

Static geo mapping files

b08ed3f

* Removed old sources, since we now pull from the internet * Renamed cross files, since they are tables * Added new mapping tables * Updated population figures * Added HHS mapping

krivard force-pushed the rf_geo_refactor branch from fd302a9 to 0c6b422 Compare September 18, 2020 18:51

Remove 8XXXX and 9XXYY, YY > 56 JHU FIPS codes, updated Puerto Rico

528ef2e

chinandrew reviewed Sep 20, 2020

View reviewed changes

_delphi_utils_python/data_proc/geomap/geo_data_proc.py Show resolved Hide resolved

chinandrew reviewed Sep 20, 2020

View reviewed changes

_delphi_utils_python/data_proc/geomap/geo_data_proc.py Outdated Show resolved Hide resolved

chinandrew reviewed Sep 20, 2020

View reviewed changes

_delphi_utils_python/delphi_utils/geomap.py Outdated Show resolved Hide resolved

chinandrew reviewed Sep 20, 2020

View reviewed changes

_delphi_utils_python/delphi_utils/geomap.py Outdated Show resolved Hide resolved

chinandrew reviewed Sep 20, 2020

View reviewed changes

Fix too long lines

8689a8a

chinandrew reviewed Oct 1, 2020

View reviewed changes

_delphi_utils_python/delphi_utils/geomap.py Outdated Show resolved Hide resolved

dshemetov added 9 commits October 1, 2020 00:39

A few comment fixes and additions, minor change of "is" to "=="

5f7c28c

Linting fixes for the tests

9a3ffe9

Update EMR hosp geomapper with new changes

2ddd9b1

Emr hosp update that should've been in previous commit

eccc72b

Update the jhu indicator with geomapper changes

9aacec5

A few minor changes:

0cfe78b

* Correct README information about JHU fixes * Remove JHU NYC county splitting * Add Dmitry to authorship comments in geo_data_proc and geomap

Merge branch 'rf_geo_refactor' of github.com:cmu-delphi/covidcast-ind…

cb171dc

…icators into rf_geo_refactor

Remove unneeded numpy import

8c9a143

Small update to README

3ab1871

krivard mentioned this pull request Oct 2, 2020

mod changelog to incorporate JHU geocoding cmu-delphi/delphi-epidata#231

Merged

dshemetov mentioned this pull request Oct 5, 2020

Consider supporting a national geo level for all indicators #199

Closed

dshemetov added 5 commits October 5, 2020 17:08

Modify national level code support:

4e6ce63

* Change 'national' -> 'nation' for consistency with #207 * Enforce FIPS and ZIP from_code when going to 'nation' level

Add archive bypass flag to JHU

21cb473

* if 'bucket_name' param is empty, do not archive * typo fix to comment in geo_data_proc

Merge branch 'rf_geo_refactor' of github.com:cmu-delphi/covidcast-ind…

78ffb9a

…icators into rf_geo_refactor

Add indicator testing notebook and geocoding utility demo notebook

64fb9b7

krivard approved these changes Oct 7, 2020

View reviewed changes

jhu/delphi_jhu/geo.py Outdated Show resolved Hide resolved

Remove trailing whitespace

b830cc8

krivard closed this in cmu-delphi/delphi-epidata#231 Oct 7, 2020

krivard reopened this Oct 7, 2020

krivard merged commit 8867c14 into main Oct 7, 2020

krivard mentioned this pull request Oct 7, 2020

Propagate geomapper changes into JHU #297

Merged

krivard deleted the rf_geo_refactor branch October 29, 2020 18:11

dshemetov mentioned this pull request Feb 28, 2025

NSSP data for HRRs is not summed properly #2129

Closed

Further refactoring for the geo coding utility #217

Further refactoring for the geo coding utility #217

Uh oh!

Conversation

dshemetov commented Aug 13, 2020

Uh oh!

dshemetov commented Aug 28, 2020

Uh oh!

dshemetov commented Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krivard commented Aug 28, 2020

Uh oh!

dshemetov commented Aug 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsharpna commented Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dshemetov commented Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dshemetov commented Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsharpna commented Sep 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dshemetov commented Sep 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dshemetov commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krivard commented Sep 18, 2020

Uh oh!

chinandrew Sep 20, 2020

Choose a reason for hiding this comment

Uh oh!

chinandrew Sep 20, 2020

Choose a reason for hiding this comment

Uh oh!

chinandrew commented Sep 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chinandrew Sep 20, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chinandrew Sep 20, 2020

Choose a reason for hiding this comment

Uh oh!

chinandrew commented Sep 30, 2020

Uh oh!

dshemetov commented Sep 30, 2020

Uh oh!

dshemetov commented Sep 30, 2020

Uh oh!

Uh oh!

krivard commented Oct 1, 2020

Uh oh!

krivard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dshemetov commented Aug 28, 2020 •

edited

Loading

dshemetov commented Aug 29, 2020 •

edited

Loading

jsharpna commented Sep 2, 2020 •

edited

Loading

dshemetov commented Sep 2, 2020 •

edited

Loading

dshemetov commented Sep 2, 2020 •

edited

Loading

jsharpna commented Sep 3, 2020 •

edited

Loading

dshemetov commented Sep 11, 2020 •

edited

Loading

dshemetov commented Sep 17, 2020 •

edited

Loading

chinandrew commented Sep 20, 2020 •

edited

Loading