Ap 330/feature v3

[ccleung]

WIP

[dmpe]

xts, timeSeries should also go into the @imports. Like here https://github.com/quandl/quandl-r/pull/36/files#diff-35ba4a2677442e210c23a00a5601aba3R15

[RaymondMcT]

xts needs to stay in depends as the package will break if it is moved into imports. If I move timeSeries into imports it will be automatically installed with Quandl. It is the intention to expand the return formats eventually to some of the others in R (its, irts, etc.) and I do not want them to be required installations with the package.

As well adding the lines

#' @import httr
#' @import jsonlite

Will load the entirety of httr and jsonlite into the global namespace, which I don't think is necessary.

[dmpe]

In regard to the second one: you can also use importFrom pck fnct.

-> timeseries: then it should be documented (+at least added to the
suggested ones; saw later as it is already done; sorry)

[ccleung]

@dmpe Is there a reason why you want to import the namespace directives? we are hoping to following best practices seen in other popular R packages, e.g., devtools does not import package namespaces: https://github.com/hadley/devtools/blob/master/R/github.R#L10 and we prefer not to do so as well

[dmpe]

@ccleung

Yes.
ropensci/software-review#17 (comment) (section Imports)

You have httr and jsonlite (and many others) as Imports in DESCRIPTION file, but they aren't listed in any @ import. Was there a reason behind this? Even though httr and jsonlite are namespaced, those will still fail for users that don't have those pkgs installed.

best practices seen in other popular R packages: Well, dplyr is very popular.

https://github.com/hadley/dplyr/search?utf8=%E2%9C%93&q=importFrom&type=Code
https://github.com/ropensci/geojsonio/search?utf8=%E2%9C%93&q=import&type=Code
https://github.com/ropensci/fulltext/search?utf8=%E2%9C%93&q=import