business-science / tidyquant

Bringing financial analysis to the tidyverse
https://business-science.github.io/tidyquant/
Other
856 stars 175 forks source link

Depends vs. Imports vs. Suggests #24

Closed ClaytonJY closed 7 years ago

ClaytonJY commented 7 years ago

What's the motivation for having dependencies in Depends? The modern convention is to have as much as possible in Imports instead, though there are exceptions, I'm curious why packages like quantmod have to be in Depends, while Quandl works fine in Imports.

Further, could some things be moved into Suggests, like Quandl and PerformanceAnalytics? I think there's plenty of use cases where one wouldn't need to touch functions that relate to those packages, so it would be nice to not force such users to install them in order to use tidyquant.

mdancho84 commented 7 years ago

This is a constant debate. The overriding reason we use Depends is to make available all tidyverse & lubridate functionality and all quantmod, xts, zoo, and PerformanceAnalytics functions. tidyverse and lubridate are commonly needed as part of tidyquant. Loading the quantmod, xts, zoo, and PA packages enables functions and documentation to be available for the user which helps with autocomplete and researching underlying / wrapped functions. It was my decision to make these Depends. This was the right way to go from my perspective, but I also understand where you are coming from.

ClaytonJY commented 7 years ago

Ah that makes sense; I've put dplyr in Depends for the exact same reasons, but only for non-public packages. I'd argue (Suggest?) that for a public CRAN package, it might be considered better etiquette to not load things for the user in this way.

As an example, tidytext, my go-to domain-specific extension to the tidyverse, leaves everything in Imports, despite having similar soft-dependencies on things like dplyr or in their case stringr, hunspell, etc: https://github.com/juliasilge/tidytext/blob/master/DESCRIPTION. Not saying that's better or worse, but it gives a precedent, and might suggest what user expectations are.

mdancho84 commented 7 years ago

Yep, I actually used tidytext as my go-to when developing tidyquant. Great pkg! I agree, the Depends etiquette is not the best with tidyquant. It's a balance though with trying to maximize the user ability out-of-the-box. Davis and I had this discussion a while back, and we agreed that when faced with this challenge we need to side on maximizing the user experience. The cost so far seems to be acceptable. In a perfect world, we'd make everything Suggests.

BTW, if you have any mod's you'd like to try to implement, give it a shot. I'm interested to see if there's a better method you can come up with.

pgensler commented 7 years ago

@mdancho84 any particular reason why the XLConnect package is listed in suggests for tidyquant? XLConnect imports rJava, which is not lightweight by any means, not sure the reasoning here. I would imagine if you needed it truly, maybe readxl in its place is a better alternative, but that's just my two cents.

mdancho84 commented 7 years ago

Believe me we had a lot of debate on this internally. Unfortunately XLConnect is the only package that could be used to read excel files from SPDRS, which is used in tq_index(). We could not get readxl, gdata, or any of the non-rJava packages to read the excel file properly.

mdancho84 commented 7 years ago

Here's the link to the issue: https://github.com/tidyverse/readxl/issues/372