Skip to content

Conversation

@capnrefsmmat
Copy link
Contributor

Bizarrely, even though R CMD check passed perfectly, the uses of dplyr and lubridate introduced in #275 didn't actually work in the installed package:

> library(covidcast)
> symptoms <- covidcast_signal("fb-survey", "smoothed_hh_cmnty_cli", geo_type="state")
Fetched day 2020-04-15 to 2020-06-23: 1, success, num_entries = 3601
Error in summarize(., geo_value = list(geo_value)) : 
  could not find function "summarize"
> # and after fixing that:
> foo = covidcast_signal("fb-survey", "smoothed_hh_cmnty_cli", start_day="2020-11-20", end_day="2020-11-24", geo_type="state")
Fetched day 2020-11-20 to 2020-11-24: 1, success, num_entries = 153
Error in ymd(missing_time_values) : could not find function "ymd"

This is because there was no @importFrom lubridate ymd or equivalent, or the same for dplyr, so these functions are not in the package NAMESPACE.

I've fixed dplyr here by using the dplyr:: prefix, and simply replaced the ymd() calls with a short function, since we were already using as.Date() for the same purpose in several places.

I'm not sure why the tests worked -- this is exactly the kind of problem testing should have detected. Maybe because some of the test files use dplyr, it was in the namespace, and none of the tests exercise the code that called ymd()? Dunno.

We do not import all of dplyr into our namespace, so it's necessary to
use the prefixes here.
It was not in NAMESPACE, so these ymd() calls failed, and we already had
copypasta using as.Date. Turn the copypasta into a simple function and
use it instead of needing the dependency.
@capnrefsmmat capnrefsmmat merged commit 0dd29c3 into r-pkg-devel Nov 24, 2020
@JedGrabman
Copy link
Contributor

Is there an open issue for investigating why R CMD check succeeded and if not, can you create one? We should try to prevent this sort of problem from being able to happen again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants