tigris icon indicating copy to clipboard operation
tigris copied to clipboard

Feature suggestion: Extending `lookup_code()` to return just FIPS codes (with no message)

Open elipousson opened this issue 3 years ago • 5 comments

I'm developing a package to access a federal agency API that uses integer versions of FIPS codes for state and county identifiers. I found the lookup_code() function but it didn't really do what I needed so I ended up importing the validate_state() and validate_county() functions into my own package in order to make this work.

I am curious if you'd be interested in a pull request to extend the existing lookup_code() function that makes it optionally return just FIPS codes and/or exporting the useful validate_state() and validate_county() functions. I included my own lookup_fips() function below for consideration of some possible options. I'm sure this isn't a widespread use case but figured there is no harm in asking.

# Validate state and county name/abbreviation and convert to FIPS number
lookup_fips <- function(state, county = NULL, several.ok = FALSE, list = FALSE, int = TRUE) {
  if (!several.ok) {
    state_fips <- suppressMessages(validate_state(state))
    county_fips <- suppressMessages(validate_county(state, county))
  } else {
    state_fips <- suppressMessages(vapply(state, validate_state, USE.NAMES = FALSE, FUN.VALUE = "1"))
    if (!is.null(county)) {
      county_fips <- suppressMessages(mapply(validate_county, state_fips, county, USE.NAMES = FALSE))
    } else {
      county_fips <- NULL
    }
  }

  if (int) {
    state_fips <- as.integer(state_fips)
    if (!is.null(county_fips)) {
      county_fips <- as.integer(county_fips)
    }
  }

  if (list) {
    list(
      "state" = state_fips,
      "county" = county_fips
    )
  } else if (!is.null(county_fips)) {
    county_fips
  } else {
    state_fips
  }
}

elipousson avatar Jan 31 '22 02:01 elipousson

yeah I like this idea! lookup_code() is a relic of the early days of the package when I needed a quick way to look up those codes, as it preceded validate_state() and validate_county(). I don't think there's any harm in exporting those functions though this may also be useful. Would it make sense to return a data frame, do you think?

walkerke avatar Feb 04 '22 17:02 walkerke

That makes sense! It occurs to me that returning a data frame could also allow the lookup function work a bit more broadly where the state parameter is required (supporting state name, abbreviation, or FIPS number as character or numeric) and county is optional (as name or FIPS number) but the function always returns a dataframe with columns for everything (state name, state abbreviation, state FIPS and, if county is provided, county name and county FIPS). This could make it work for translating a FIPS into an abbreviation as well as the other way around. Does that make sense?

Happy to put together a pull request with a first draft if that is helpful. I noticed the old TODO for eliminating the dependence on the fips_state_table so I could try to tackle that at the same time.

elipousson avatar Feb 05 '22 23:02 elipousson

@elipousson I'm happy to go the direction you find most useful here; one limitation about returning a data frame is that users can already just grab the fips_codes object and just filter it, so it may be more useful to do what you originally suggested or just export validate_state() and validate_county() to allow for use in a programmatic sense.

walkerke avatar Feb 19 '22 17:02 walkerke

I think I came up with an approach that works pretty well. It maintains the existing functionality but adds the option to set .msg = FALSE for lookup_codes and force it to return a data frame instead of messages.

One small question: I used vapply to get validate_county and validate_state to support multiple states or a state and multiple counties but did not set USE.NAMES = FALSE. Would an unnamed vector be preferable or does the approach in my commit work OK?

elipousson avatar Feb 27 '22 05:02 elipousson

No real rush, @walkerke but I just wanted to ping you in case this got lost in the shuffle. Let me know if I should should go ahead and make a pull request for your review or if you'd suggest a different approach!

elipousson avatar Mar 22 '22 21:03 elipousson