Description
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:
const MSG_MODEL_EXPECTED = "Type encountered where model instance expected"
function check_is_instance(model, message=MSG_MODEL_EXPECTED)
model isa Model || throw(AssertionError(message)) # in future, use `ismodel(model)`
return nothing
end cc @rikhuijzer