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

possible to filter by multiple years? #13

Open
zackarno opened this issue Jun 27, 2024 · 3 comments
Open

possible to filter by multiple years? #13

zackarno opened this issue Jun 27, 2024 · 3 comments

Comments

@zackarno
Copy link
Collaborator

Just had a use-case where I wanted to filter ipc_get_areas() by multiple years.

Seems only possible for single years.

Snipped from ipc_get_areas(), but if possible I would imagine logic could be applied to other ipc_get calls as well if

ripc/R/ipc_get_areas.R

Lines 32 to 35 in fa404d0

#' @param country ISO2 country code.
#' @param year Single numeric year to filter analysis, calculated from the
#' maximum year of current period dates. If `NULL`, the default, returns
#' data for all years.

@caldwellst
Copy link
Contributor

So, could be nice to include this functionality. However, I have a concern on the backend for allowing multiple passing of parameters. Say you wanted to allow multiple country and year values. These cases would be easy:

ipc_get_areas(country = "AF", year = 2020)
ipc_get_areas(country = "AF", year = 2020:2022)
ipc_get_areas(country = c("SO", "AF"), year = 2020)

That is because in the underlying script, we could simply use a purrr::map() style call passing in those vectors. However, if you had mismatched lengths of vectors, you'd have to do something different. Maybe using tidyr::complete() to get all combinations of the values. For instance:

ipc_get_areas(country = c("SO", "AF"), year = 2020:2023)

That would fail with any basic purrr::map() setup. At this stage, given that we can just return all values and manually filter after, wondering if the complexity is worth it?

@zackarno
Copy link
Collaborator Author

Yea probably not worth it. The user has good options for post-filtering.

What do you think about just wrapping the filter call - I know it's not elegant or effecient, but still might improve user-experience and I doubt the majority of users would really notice the lag

something to this effect:

ipc_get_areas <- function(..., year){
  if( !is.null(year) & length(year)>1){
       ipc_get_areas(...) |>
          filter(year %in% in year
}

@caldwellst
Copy link
Contributor

So looking into this, again, could be quite complex. For ipc_get_areas(), to have a consistent API, we would want to also handle multiple passes of other parameters: country, period, even id, alongside year. Is possible, but I am not so sure about putting this functionality directly in the package which already does some complex wrangling, and trying to just keep it mirroring the API as much as possible. What do you think Zack? Any thoughts @hannahker?

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