Docs review #149
Docs review #149
Conversation
It's not exported and doesn't seem to be intended for users.
The argument is called source, not data_source. Also, fix broken URL
| @@ -32,10 +32,10 @@ parse_signal <- function(signal, base_url) { | |||
| } | |||
|
|
|||
| #' @export | |||
| print.covidcast_data_signal <- function(signal, ...) { | |||
There was a problem hiding this comment.
Not sure why you're preferring a more generic variable name?
There was a problem hiding this comment.
We're required to use x, because that's how print() is defined in base R:
> print
function (x, ...)
UseMethod("print")
See https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Generic-functions-and-methods
This is checked by R CMD check, which issues warnings when the argument names don't match.
R/epidatr-package.R
Outdated
There was a problem hiding this comment.
What a peculiar bit of boilerplate. I'll definitely be taking your word for it
There was a problem hiding this comment.
There was a problem hiding this comment.
Was also curious, thanks!
| @@ -14,18 +14,49 @@ library(epidatr) | |||
| library(dplyr) | |||
| ``` | |||
|
|
|||
| The epidatr package provides access to all the endpoints of the [Delphi Epidata | |||
There was a problem hiding this comment.
definitely needed to be less sparse
|
Pushed a fix for the error message saying |
There was a problem hiding this comment.
Thank you for this pass Alex, overall looks great.
- Made one typo change
- We tentatively decided to keep
covidcast_epidata()private because we weren't sure if the interface was going to change. I'm fine with re-exporting it, merging it in for now, and discussing it tomorrow. Can always revert later.
Here's an initial batch of changes from reading through the package documentation this morning, and running
devtools::check(remote=TRUE). Each commit is small and the message should explain the purpose, so it may be easier to review each commit separately.