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

Idea for issue #17 about ambiguous error #37

Closed
wants to merge 1 commit into from

Conversation

e3bo
Copy link

@e3bo e3bo commented Jul 25, 2022

One way to look at the error described in #17 is that it occurs because read_csv interprets its argument as a filename instead of literal data. Simply wrapping the argument of read_csv within I can prevent that, as described in the docs here.

With this change the test case of a call that produces a null result

y <- covidcast( 
      data_source = "bad-source", 
      signals = "bad-signal", 
      geo_type = "state",  
      time_type = "day", 
      time_value = epirange(20200601, 20221201),
      geo_values = "ca,fl")

produces a

> y %>% fetch_tbl()
# A tibble: 0 × 15
# … with 15 variables: X1 <chr>, X2 <chr>, X3 <chr>, X4 <chr>, X5 <chr>, X6 <chr>, X7 <chr>, X8 <chr>, X9 <chr>, X10 <chr>,
#   X11 <chr>, X12 <chr>, X13 <chr>, X14 <chr>, X15 <chr>
# ℹ Use `colnames()` to see all variable names
Warning message:
The following named parsers don't match the column names: source, signal, geo_type, geo_value, time_type, time_value, issue, lag, value, stderr, sample_size, direction, missing_value, missing_stderr, missing_sample_size

At least the situation is clearer.

@brookslogan
Copy link
Contributor

Thanks for the idea! Looks like vroom (which backs read_csv) will now require this I for literal data. So this looks like it solves #60.

To fully address #17, I think we still need something more explicit; getting this 0-row result with weird column names is also confusing. I think in the old epidata client, we had an error response code for this (which users could then check and raise R errors for). Probably makes sense to just throw an error if the response contains no data.

@dshemetov
Copy link
Contributor

#17 has now been addressed by changes in #99, so this change will not be needed. We may consider adding I(response_content) into fetch_csv at a later time (I am not familiar with the behavior of I() enough to say whether this is needed in general), but since that function is no longer exported, this isn't critical.

I still want to thank you for your contribution though. This I() function is really handy to know about 🙏

@dshemetov dshemetov closed this May 25, 2023
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

Successfully merging this pull request may close these issues.

3 participants