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

Don't load lubridate on startup + other upkeep #237

Merged
merged 9 commits into from
Oct 3, 2023
Merged

Don't load lubridate on startup + other upkeep #237

merged 9 commits into from
Oct 3, 2023

Conversation

olivroy
Copy link
Contributor

@olivroy olivroy commented Oct 2, 2023

This should fix #116 , supersedes #163 (I think it's better to rely on cli's inline theme, rather than RStudio, as it makes the package more portable.

Using how tidyverse loads its packages rather than base R to create less clutter when loading the package.

Removing packages from depends

You will note that I tweaked the vignettes loading scheme.

Showing library(tidyverse), library(tidyquant)

actually running library(dplyr) library(lubridate) etc. library(tidyquant)

I also added skip() to avoid test failures when APIs are not available.

Move tq_performance() and tq_transmute() lists of functions into an internal data, as I assume that xts, quantmod are quite stable and if there are any new functions, it is always possible to re-run the data-raw script and update the possible functions.

Addresses #235

Edit: closes #91

@olivroy
Copy link
Contributor Author

olivroy commented Oct 2, 2023

I also changed xlim and ylim so that the example doesn't show as blank.
https://business-science.github.io/tidyquant/reference/geom_ma.html

Do you know why the 75-125 lims are not correct anymore?

eb5099a#diff-c53cfe3cb0db9e7cb411222ea0b11a7dd25ed649bd6e7ad9bce1b66f73abe0fc
they seemed correct here?

@olivroy olivroy changed the title Don't load lubridate on startup Don't load lubridate on startup + other upkeep Oct 2, 2023
@olivroy
Copy link
Contributor Author

olivroy commented Oct 3, 2023

@mdancho84 let me know if you think these changes are too agressive!

But I think there is an added value here as there are less messages on startup, and the new startup messages are more familiar for tidyversee users)

@mdancho84 mdancho84 merged commit f46fd6e into business-science:master Oct 3, 2023
7 checks passed
@mdancho84
Copy link
Collaborator

Looks good to me. I felt it was an improvement and simplification. Plus fewer dependencies too.

@olivroy olivroy deleted the lubridate branch October 3, 2023 17:25
@mdancho84
Copy link
Collaborator

Let me know if you want to help out on some other things. Shoot me an email at [email protected]

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.

Compatibility with 'import' package?
2 participants