mlr-org / mlr3misc

Miscellaneous helper functions for mlr3
https://mlr3misc.mlr-org.com
GNU Lesser General Public License v3.0
11 stars 2 forks source link

About how `$mget()` handles multiple inputs #23

Closed pat-s closed 5 years ago

pat-s commented 5 years ago

To prevent the following

library(mlr3learners)
library(mlr3)

foo = mlr_learners$mget("classif.rpart", "classif.svm")

length(foo)
#> [1] 1

Created on 2019-07-29 by the reprex package (v0.3.0)

In this case passing the learners as a vector is required. Since the possible ellipsis in mget() usually needs to be named, we could somehow account for cases like above and maybe even get rid of the c() wrapping?

mb706 commented 5 years ago

unnamed arguments that are not the first argument first should probably throw an error. Not sure about getting rid of c()

mb706 commented 5 years ago

I think making $mget("classif.rpart", "classif.svm") work would be cool, but I know this is the kind of aesthetic judgement that @berndbischl usually disagrees with me on ;-)

mllg commented 5 years ago

I can throw an error if any argument in ... is not named to protect from dumb mistakes. But beyond that, it is just sugar and wild guessing what the user meant to do.

berndbischl commented 5 years ago

Agreeing with Michel

pat-s commented 5 years ago

Ok. Sugar would have been nice here.

Forgetting the c() in mget() is not really a dumb mistake imo but can happen naturally as many functions support this syntax. What is important here is that we at least thrown an error - what you currently get is an unexpected return instead of an error - this is hard to catch sometimes.

mllg commented 5 years ago

You now get an exception if you pass > 1 unnamed args to mget.