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

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

Open
elipousson opened this issue Jan 31, 2022 · 5 comments

Comments

@elipousson
Copy link

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
  }
}
@walkerke
Copy link
Owner

walkerke commented Feb 4, 2022

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?

@elipousson
Copy link
Author

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.

@walkerke
Copy link
Owner

@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.

@elipousson
Copy link
Author

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
Copy link
Author

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 added a commit to elipousson/tigris that referenced this issue May 21, 2024
Also fix issue w/ how validate_county would warn in case of invalid inputs

Also fix typo (used error_call instead of caller_env) and add `check_tigris_arg()` helper function
elipousson added a commit to elipousson/tigris that referenced this issue May 26, 2024
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

No branches or pull requests

2 participants