Base the Model Registry on constructors, rather than model types; and ancillary changes #558
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this PR we:
Provide a mechanism for including wrappers, such as
TunedModel
, in the Model Registry by adding a new optionwrappers=false
tomodels(...)
method. Thefalse
fallback ensures no breakages for normal usage. In generating the model registry, usewrappers=true
to ensure wrappers are now included.Change the model registry generation process so that
name
is the always name of a constructor rather than the name of a model type, to support Enable entry of model wrappers into the MLJ Model Registry MLJ.jl#1125. For regular models, there is no difference, but for wrappers (previously excluded from the registry) there is. For example,TunedModel
is the constructor forProbabilisticTunedModel
andDeterministicTunedModel
types, but while the constructor is user-facing, the types are not. So, in the updated registry, onlyTunedModel
should appear.Add FeatureSelection.jl models to the Model Registry
Remove MLJModels.FeatureSelector src code, as this model has been migrated to FeatureSelection.jl
Update the ModelRegistry again to remove MLJModels.FeatureSelector from the registry
The last three items render #556 redundant (that PR has been merged into the current one)
This PR has been tested locally with a planned downstream update to MLJ.jl.
cc @OkonSamuel @EssamWisam