Skip to content

with_base_url sounds like it follows the withr/etc. pattern but doesn't #82

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

Closed
brookslogan opened this issue Apr 11, 2023 · 1 comment · Fixed by #156
Closed

with_base_url sounds like it follows the withr/etc. pattern but doesn't #82

brookslogan opened this issue Apr 11, 2023 · 1 comment · Fixed by #156
Labels
P2 low priority

Comments

@brookslogan
Copy link
Contributor

Some R packages have a pattern for with_* functions where they will try to run some code with some setup and mandatory cleanup, similar to the Python with statement. with_base_url sounds very much like withr::with_options, etc. It'd be nice to either follow this with-statement-like pattern or find another name for this function.

@brookslogan brookslogan added the P2 low priority label Apr 11, 2023
@brookslogan brookslogan changed the title with_base_url sounds like it might follow the withr/httr/httptest/testthat/etc. pattern but doesn't with_base_url sounds like it might follow the withr/etc. pattern but doesn't Apr 11, 2023
@brookslogan brookslogan changed the title with_base_url sounds like it might follow the withr/etc. pattern but doesn't with_base_url sounds like it follows the withr/etc. pattern but doesn't Apr 11, 2023
@dshemetov
Copy link
Contributor

dshemetov commented Apr 11, 2023

So this function is intended to work like this

covidcast("fb-survey", "smoothed_cli", "day", "nation", epirange(20210405, 20210410), "us") %>%
  with_base_url("https://staging.delphi.cmu.edu/epidata/") %>% 
  fetch_df()

I'm not used to withr, so this didn't throw me off. Is there an interface change we can make to make it more intuitive to withr users? Something like

with_base_url("https://staging.delphi.cmu.edu/epidata/", 
  covidcast("fb-survey", "smoothed_cli", "day", "nation", epirange(20210405, 20210410), "us")
) %>%
  fetch_df()

covidcast("fb-survey", "smoothed_cli", "day", "nation", epirange(20210405, 20210410), "us") %>%
  with_base_url("https://staging.delphi.cmu.edu/epidata/", .) %>%
  fetch_df()

and with_base_url will modify the global_base_url variable inherited from constants.R?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 low priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants