Closed topepo closed 4 years ago
Breaking API is really bad unless necessary for some major reason. A lot of this could be accomplished without breaking any backwards compatibility.
metric
from sbf()
and rfe()
to the control function would break consistency with its location in train()
. Alternatively, backwards compatibility could be maintained by adding this to the control functions and still keeping it where it is. Duplication like this isn't so great, however.method
, number
, and repeats
are some of the most frequently used options in trainControl()
. While this would add consistency of grouping them together into a list (like adaptive
), it becomes more verbose. At the this point, it encourages the user to define a list outside of trainControl()
instead of using it inline (much like using defining trainControl()
outside of train()
instead of using it inline), so that's not all bad. How about a backwards compatible change? The arguments number
, repeats
, etc remain and are used unless a list is passed to the argument resample
? I'm not so concerned with backwards compatibility. Considering the future users that benefit from the changes is better. Constantly worrying about backwards compatibility leads to too many controls in the code and makes debugging worse. Besides, nowadays you have the checkpoint package. This is what I started using for production code. This keeps packages to the same version as when I finished the project. No more worrying about something breaking because someone made a change to a package.
...
option in grid
. That saves me, and probably a lot of others, building custom models just for extending the grid parameters. Combined with the existing adaptive resampling
or combining it with baysian optimization might lead to some interesting parameter tuning results....
will lead to a reduction in the number of models available in caret. Which again helps in future maintenance.trainControl
and shows which pieces belong together. It will probably lead to people defining these parts outside trainControl
but that is probably not a bad thing. It is what I tend to do if I have these kind of possibilities. preProcess
options in train
to trainControl
. Then all preprocessing is defined in one place. All in all it sounds like a lot of work. Might be a good business case for the R consortium.
Not an issue but a running list of things that we might change in a single release that would break backward compatibility (which I have almost always avoided).
Suggestions and comments are welcome...
preProcess
options are passed fromtrain
(via thepreProcess
argument totrain
as opposed to being intrainControl
)metric
argument insbf
,rfe
and other feature selection routines to their control objectsgrid
module...
to thegrid
functionIt might be cleaner to change this to something like:
Overall, it would like like: