Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor of @load #340

Merged
merged 3 commits into from
Nov 27, 2020
Merged

Refactor of @load #340

merged 3 commits into from
Nov 27, 2020

Conversation

ablaom
Copy link
Member

@ablaom ablaom commented Nov 9, 2020

Addresses JuliaAI/MLJ.jl#693 .

  • The intended behaviour of @load is not changed by this PR with this exception: By default verbosity=1, so that newcomers understand what the macros is doing. I think this has been a source of confusion a number of times.
julia> @load AdaBoostStumpClassifier
[ Info: For silent loading, specify `verbosity=0`. 
import MLJDecisionTreeInterface ✔
const AdaBoostStumpClassifier = MLJDecisionTreeInterface.AdaBoostStumpClassifier ✔
AdaBoostStumpClassifier(
    n_iter = 10,
    pdf_smoothing = 0.0) @036
  • to load code without returning an instance of the new model type, there is a new macro @loadcode

  • Support for the load function is removed. Instead, for explicit importing of model types without a macro, the user may extract the appropriate load path using the load_path function, a model trait now overloaded to work on strings and model registry entries:

julia> load_path("PCA")
"MLJMultivariateStatsInterface.PCA"

julia> load_path("LinearRegressor", pkg="GLM")
"MLJModels.GLM_.LinearRegressor"

julia> load_path(models()[1])
"MLJScikitLearnInterface.ARDRegressor"

TODO:

@ablaom
Copy link
Member Author

ablaom commented Nov 9, 2020

@DilumAluthge @ExpandingMan

@ExpandingMan
Copy link

I was thinking maybe instead of @loadcode it can be called @import? It seems appropriate since the behavior is closely aligned with the functionality of the actual Julia keyword import, but perhaps using up the namespace that much is a bit aggressive.

@DilumAluthge
Copy link
Member

@import seems fine to me.

@ablaom
Copy link
Member Author

ablaom commented Nov 10, 2020

@import it is then It would seem this is not allowed??

julia> macro import() quote end end
ERROR: syntax: invalid name "import"
Stacktrace:
 [1] top-level scope at none:1

@OkonSamuel
Copy link
Member

@import it is then It would seem this is not allowed??

julia> macro import() quote end end
ERROR: syntax: invalid name "import"
Stacktrace:
 [1] top-level scope at none:1

@ablaom @loadcode sounds a little off, what of renaming to @loadmodel or @ml_import

@ExpandingMan
Copy link

Hm, yeah it might protect the keyword import. It was probably a bad idea to shadow it in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants