mlr-org / mlr

Machine Learning in R
https://mlr.mlr-org.com
Other
1.64k stars 404 forks source link

recheck listLearners refactoring by michel #774

Closed berndbischl closed 8 years ago

berndbischl commented 8 years ago

might be unfinished in little details

schiffner commented 8 years ago

Here is a detail I noticed in the tutorial:

listLearners returns NA in column note instead of "" if the note arg was not used when defining the learner.

Example:

library(mlr)

lrns = listLearners("regr")
lrns[1:3, c("class", "note")]

        class                                   note
1 regr.avNNet `size` has been set to `3` by default.
2  regr.bcart                                   <NA>
3    regr.bdk                                    

Compare https://github.com/mlr-org/mlr/blob/master/R/RLearner_regr_bcart.R and https://github.com/mlr-org/mlr/blob/master/R/RLearner_regr_bdk.R#L22

berndbischl commented 8 years ago

so we have an NA if the note is not defined? Isnt that actually good? to have a value encode this precisely?

larskotthoff commented 8 years ago

Agreed, this seems to be the better option. Then we have to remove all those empty notes from learners though.

berndbischl commented 8 years ago

Then we have to remove all those empty notes from learners though

Where are they? they should not be there in the first place?

larskotthoff commented 8 years ago

Here's a list:

RLearner_classif_dcSVM.R RLearner_classif_extraTrees.R RLearner_classif_fnn.R RLearner_classif_geoDA.R RLearner_classif_hdrda.R RLearner_classif_IBk.R RLearner_classif_kknn.R RLearner_classif_knn.R RLearner_classif_LiblineaRL1L2SVC.R RLearner_classif_LiblineaRL1LogReg.R RLearner_classif_LiblineaRL2L1SVC.R RLearner_classif_LiblineaRMultiClassSVC.R RLearner_classif_lvq1.R RLearner_classif_mlp.R RLearner_classif_multinom.R RLearner_classif_naiveBayes.R RLearner_classif_nodeHarvest.R RLearner_classif_plsdaCaret.R RLearner_classif_quaDA.R RLearner_classif_rFerns.R RLearner_classif_rrlda.R RLearner_classif_sda.R RLearner_classif_svm.R RLearner_classif_xyf.R RLearner_multilabel_rFerns.R RLearner_regr_brnn.R RLearner_regr_crs.R RLearner_regr_cubist.R RLearner_regr_earth.R RLearner_regr_extraTrees.R RLearner_regr_fnn.R RLearner_regr_frbs.R RLearner_regr_IBk.R RLearner_regr_kknn.R RLearner_regr_lm.R RLearner_regr_mars.R RLearner_regr_mob.R RLearner_regr_nodeHarvest.R RLearner_regr_penalized_lasso.R RLearner_regr_penalized_ridge.R RLearner_regr_plsr.R RLearner_regr_svm.R RLearner_regr_xyf.R RLearner_surv_coxph.R

Is it documented somewhere that this shouldn't be in there?

berndbischl commented 8 years ago

Is it documented somewhere that this shouldn't be in there?

not really, but this is manually setting a default, which is already there. can you simply remove these lines, please?

larskotthoff commented 8 years ago

sigh I should have known that you would ask that. #830

berndbischl commented 8 years ago

sorry.... and thx

schiffner commented 8 years ago

Many thx, Lars and Bernd.

so we have an NA if the note is not defined? Isnt that actually good? to have a value encode this precisely?

I'm sorry, but in my opinion: Not at all

berndbischl commented 8 years ago

fixed in #936 by @mllg thx