search

mlr-org / mlr3extralearners

Extra learners for use in mlr3.
https://mlr3extralearners.mlr-org.com/
89 stars 48 forks source link

Additional RWeka learners #324

Closed damirpolat closed 7 months ago

damirpolat commented 10 months ago

Adding 10 new learners from RWeka that I need for an automl project:

Also added tests and documentation. Any feedback is welcome!

sebffischer commented 10 months ago

The first feedback would be to make sure that the CI passes :) I guess as all these learners are so similar it might make sense to introduce a class for them and inherit from them?

damirpolat commented 10 months ago

The first feedback would be to make sure that the CI passes :) I guess as all these learners are so similar it might make sense to introduce a class for them and inherit from them?

I don't know if it makes sense to create a new class since newly added learners are no different from the rest in terms of functionality. The only difference is that most call RWeka::make_Weka_classifier('<weka/learner_name>') instead of calling them directly since they are not directly exported.

damirpolat commented 10 months ago

Final list of new learners:

sebffischer commented 10 months ago

I don't understand the argument. If all learners are almost identical, shouldn't that be a reason to introduce inheritance? probably this would have also been the right call for the other weka learners.

damirpolat commented 9 months ago

I don't understand the argument. If all learners are almost identical, shouldn't that be a reason to introduce inheritance? probably this would have also been the right call for the other weka learners.

Yes, you're right. I factored train/predict out into helper functions.

sebffischer commented 9 months ago

I hope I am not being too nitpicky!