Skip to content
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

Add PR to fips<>name and territories to fips<>abbr/ fips<>name to R package #496

Merged
merged 4 commits into from
Mar 19, 2021

Conversation

chinandrew
Copy link
Collaborator

@chinandrew chinandrew commented Mar 18, 2021

Addresses #446 and #461 for R package

Since the PR and territories data comes from multiple xlsx files in different formats, as well as some web pages for name/fips mappings, I constructed individual rda files by hand for PR and territories that are stored in data-raw/ and joined with the data pulled from the census when Make.R is run. Similar pattern to how the geomapper in delphi_utils generates the mapping files.

> library(covidcast)
> covidcast::fips_to_name("72000")
        72000 
"Puerto Rico" 
> covidcast::fips_to_name("72001")
     72001 
"Adjuntas" 
> covidcast::name_to_fips("Puerto Rico")
Puerto Rico 
    "72000" 
> covidcast::fips_to_abbr("66")
66000 
 "GU" 
> covidcast::fips_to_abbr("78")
78000 
 "VI" 
> covidcast::abbr_to_fips("MP")
     MP 
"69000" 
> covidcast::abbr_to_fips("VI")
     VI 
"78000" 

Have not rebuilt documentation yet, if that should go in this PR let me know and I'll add.

@chinandrew chinandrew requested a review from sgsmob as a code owner March 18, 2021 18:44
@chinandrew chinandrew changed the base branch from main to r-pkg-devel March 18, 2021 18:45
@capnrefsmmat
Copy link
Contributor

Hmm. So county_census is generated here if we ever need to build it from scratch. Should we add this in there? And then also build in the subsetting @statsmaths made in #493?

I kind of forgot we had the raw data generated like this, but I guess we should bring the generator up to date. @statsmaths, if you could help that'd be great.

@chinandrew
Copy link
Collaborator Author

Hmm. So county_census is generated here if we ever need to build it from scratch. Should we add this in there? And then also build in the subsetting @statsmaths made in #493?

I kind of forgot we had the raw data generated like this, but I guess we should bring the generator up to date. @statsmaths, if you could help that'd be great.

Oh I wasn't aware of that. I had to pull 2 xlsx files, one for pop/name and one for name/fips, and join them to get the df, which I did by hand. I can write up a script to do it and add to that file if that's preferred.

US territories is even more convoluted, I think each territory had its own xlsx file without fips and I pulled the name <> fips from wikipedia (since the names aren't always consistent and. this .gov site doesnt use the same county names as the census)

@capnrefsmmat
Copy link
Contributor

Yeah, I think having it all in that file would be great, and we could then add some code comments so we remember that the data is generated by the file. It'd be great if the Census data we pulled could just be extended with a few territory entries, instead of needing to merge a bunch of different things.

@chinandrew
Copy link
Collaborator Author

Alternatively I could do what I did for the python geomapper and hard code the numbers into the data generation script: https://github.com/cmu-delphi/covidcast-indicators/pull/668/files

@chinandrew
Copy link
Collaborator Author

And then also build in the subsetting @statsmaths made in #493?

Also what's the subsetting you're referring to here?

@capnrefsmmat
Copy link
Contributor

Alternatively I could do what I did for the python geomapper and hard code the numbers into the data generation script: https://github.com/cmu-delphi/covidcast-indicators/pull/668/files

I think that makes sense

Also what's the subsetting you're referring to here?

Taylor subset the columns in county_census so it didn't have as many columns as the script generates, since most of them weren't needed

@chinandrew chinandrew changed the title Add Puerto Rico to fips/name_to_name/fips Add PR to fips<>name and territories to fips<>abbr/ fips<>name Mar 18, 2021
@chinandrew chinandrew requested a review from sgsmob March 18, 2021 22:50
@chinandrew
Copy link
Collaborator Author

Ok updated this PR to include territories and add the code to Make.R. I put the hand-constructed territories data into rda files within the data-raw folder.

Ready for another review @capnrefsmmat @sgsmob

@chinandrew chinandrew changed the title Add PR to fips<>name and territories to fips<>abbr/ fips<>name Add PR to fips<>name and territories to fips<>abbr/ fips<>name to R package Mar 18, 2021
@statsmaths
Copy link

This looks great. Once you merge, I will also add the code I have to create the smaller geojson files into the data-raw directory.

Copy link
Contributor

@capnrefsmmat capnrefsmmat left a comment

Choose a reason for hiding this comment

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

looks good

@capnrefsmmat capnrefsmmat merged commit 0dc7ff4 into r-pkg-devel Mar 19, 2021
@statsmaths statsmaths deleted the add-pr branch March 19, 2021 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants