8000 Forward extra arguments to `caret::train()` by kelly-sovacool · Pull Request #304 · SchlossLab/mikropml · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Forward extra arguments to caret::train() #304

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

Merged
merged 21 commits into from
Oct 15, 2022
Merged

Forward extra arguments to caret::train() #304

merged 21 commits into from
Oct 15, 2022

Conversation

kelly-sovacool
Copy link
Member
@kelly-sovacool kelly-sovacool commented Oct 14, 2022

run_ml() now forwards extra arguments (...) to train_model(), which then forwards them to caret::train(). This allows our users to take advantage of specialty arguments used by specific model engines such as ntree for rf and weights for glmnet (among other models that accept case weights), even when we haven't explicitly defined them in run_ml().

To implement this, I had to switch from formula notation to x,y notation. We had wanted to do that anyway to prevent #156. However, formula notation is still used for support vector machines to circumvent a problem in kernlab (see this comment in caret iss #809).

Issues

Change(s) made

  • Forward ellipsis (...) to caret::train().
  • Removed ntree argument from run_ml() and train_model(), since that is now captured by ....
  • Add unit tests to make sure that the weights argument is used when given for glmnet.
  • Changed logic of train_model().
    • Use formula notation with caret::train() for svm models.
    • Use x,y notation with caret::train() for all others.

Checklist

(Strikethrough any points that are not applicable.)

  • Write unit tests for any new functionality or bug fixes.
  • Update docs if there are any API changes:
    • roxygen comments
    • vignettes
      • Update wording to reflect that ntree is not one of the named arguments in run_ml() but can still be defined and used for rf.
      • Write an example showing how to use case weights.
  • Update NEWS.md if this includes any user-facing changes.
  • The check workflow succeeds on your most recent commit. This is always required before the PR can be merged.

@kelly-sovacool kelly-sovacool changed the title Refactor run_ml() & train_model() to forward extra arguments to caret::train() Forward extra arguments to caret::train() Oct 14, 2022
@codecov-commenter
Copy link
codecov-commenter commented Oct 14, 2022

Codecov Report

Base: 98.09% // Head: 98.09% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (019cf20) compared to base (2b07e51).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #304   +/-   ##
=======================================
  Coverage   98.09%   98.09%           
=======================================
  Files          13       13           
  Lines         995      999    +4     
=======================================
+ Hits          976      980    +4     
  Misses         19       19           
Impacted Files Coverage Δ
R/checks.R 100.00% <100.00%> (ø)
R/run_ml.R 100.00% <100.00%> (ø)
R/train_model.R 100.00% <100.00%> (ø)
R/utils.R 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kelly-sovacool kelly-sovacool marked this pull request as ready for review October 15, 2022 01:44
@kelly-sovacool kelly-sovacool requested review from a team and sklucas and removed request for a team October 15, 2022 01:44
@kelly-sovacool kelly-sovacool merged commit 833712c into main Oct 15, 2022
@kelly-sovacool kelly-sovacool deleted the iss-271 branch October 15, 2022 02:40
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.

Providing case weights Update from model formula to x,y notation in train function
3 participants
0