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

Importing `recipes` introduces many implicit dependencies #48

Closed
echasnovski opened this issue Aug 3, 2018 · 3 comments
Closed

Importing `recipes` introduces many implicit dependencies #48

echasnovski opened this issue Aug 3, 2018 · 3 comments

Comments

@echasnovski
Copy link

@echasnovski echasnovski commented Aug 3, 2018

At current moment, installation of rsample (supposing tidyverse is installed) causes many packages to be installed as dependencies of recipes. I had 20 extra packages installed (excluding recipes itself):

List of extra packages ‘numDeriv’, ‘SQUAREM’, ‘abind’, ‘lava’, ‘kernlab’, ‘CVST’, ‘DEoptimR’, ‘magic’, ‘prodlim’, ‘DRR’, ‘robustbase’, ‘sfsmisc’, ‘geometry’, ‘ipred’, ‘dimRed’, ‘timeDate’, ‘ddalpha’, ‘gower’, ‘RcppRoll’, ‘pls’

I understand the reason to facilitate using the recipes with rsample by providing prepper() wrapper, but I think 20 packages not explicitly needed for core functionality is a little bit too much for one simple wrapper. Maybe it is a good idea to move recipes back to Suggests, mention it with wrapper in vignette, and possibly add rsample into recipes Import (as it already has many imported packages)? Keep rsample more light-weight?

Also current situation might be inconvenient for package authors willing to import rsample.

@topepo
Copy link
Collaborator

@topepo topepo commented Aug 3, 2018

It would make sense to move prepper to recipes (which would import rsample). I'll work on that.

topepo added a commit that referenced this issue Aug 4, 2018
topepo added a commit that referenced this issue Aug 4, 2018
topepo added a commit that referenced this issue Aug 4, 2018
@ClaytonJY
Copy link
Contributor

@ClaytonJY ClaytonJY commented Sep 11, 2018

@topepo would it also make sense to move form_pred to recipes? Seems a bit out-of-place here and doesn't seem to be used internally at all.

@topepo
Copy link
Collaborator

@topepo topepo commented Nov 17, 2018

I'm going to leave that one in rsample. You don't really need it for recipes and it is just a little function to help with training materials.

@topepo topepo closed this Nov 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.