Open ablaom opened 2 years ago
I've made some progress here by adding a check_ismodel
method to MLJBase #819 and implementing it for Stack
.
Left to do is to implement the check in:
(For now pipelines allow types to be used in place of instances, which I think was a bad idea, but changing that now would be breaking.)
A very common gotcha for users (not necessarily beginners) is calling a method with a model type instead of a model instance. Sometimes the consequence is a confusing error message.
Sometimes we check this (eg, here) but sometimes we don't (eg, base models in a
Stack
).In some cases (eg, pipelines) we catch a type and automatically instantiate a default instance, if necessary. I have mixed feelings about this. For example, some model wrappers, eg, TunedModel, EnsembleModel, cannot be called with zero arguments (an atomic model must be specified). So I propose we eventually deprecate this behaviour
For now, I would like a review of MLJBase.jl which introduces the check everywhere a user is expected to call a method with an instance, but might supply a type by accident. An informative error is to be thrown. This could start with some infrastructure along the lines of:
cc @rikhuijzer