-
Notifications
You must be signed in to change notification settings - Fork 2
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
Remove tidyverse dependencies? #27
Comments
Native pipe would restrict you to R>=4.1.0. |
Yes, so I tend to go for some old-style coding here. In theory, CRAN only supports the last two versions for binary builds. So at some point, we should be able to assume 4.1, but I don't think we're quite there yet. |
For icd_lookup() and icd_browse(), the base R versions seems to be considerably quicker (see commit "ba27277406ec5394eab9a311ec5b99ed565fd24c"):
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Dependency on dplyr, purrr and magrittr was primarily for convenience and simplicity. It is assumed that most users have these packages installed already, but package check (CI) can take longer than necessary because dependencies need to be installed.
It should be possible to replace tidyverse dependencies with base R, or even data.table (which might also bring big performance improvements).
This affects primarily icd_expand() and icd_history().
There is also no reason to keep the magrittr pipe. This could either be replaced with the new native pipe, or coded "old-style".
Existing tests should ensure that such a rewrite does not introduce bugs.
The text was updated successfully, but these errors were encountered: