Add the optional install_pkgs keyword argument to the @load macro

[DilumAluthge]

This pull request adds the optional install_pkgs keyword argument to the @load macro.

If install_pkgs is true, then any necessary packages will automatically be installed.

The default value is install_pkgs=false.

Example usage:

julia> using MLJModels

julia> @load NeuralNetworkClassifier install_pkgs=true

Motivation

@juliohm has brought up that installing all of the necessary packages is one of the pain points for Julia beginners when they first start out using MLJ.

Therefore, we can tell these beginner users to use install_pkgs=true, and then the necessary packages will automatically be installed for them.

[ablaom]

I really like this proposal.

@DilumAluthge In case install_packages is false and the package is not in the env, could we catch the error and issue a suggestion "Specify install_packages=true to automatically add the necessary package to your current environment"

[ablaom]

Maybe for consistency with pkg=... option, we should call this new option install_pkg?? There would only be one pkg to install.

[juliohm]

Alternatively one could show a yes/no option to the user of the package is not installed. That would help with interactive sessions.

…

On Mon, Nov 30, 2020, 00:52 Anthony Blaom, PhD ***@***.***> wrote: Maybe for consistency with pkg=... option, we should call this new option install_pkgs?? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#346 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZQW3M2FIIZT7U2AXHPZFDSSMJJVANCNFSM4UGJARDA> .

[ablaom]

@juliohm Not sure I understand the suggestion here. I'm certainly not keen on interactive behaviour by default, if that is what you are suggesting. We could instead have the flag interactive=true, but that would add one more step (the user needing to answer the question "Do you want to install...?"), no?

[juliohm]

I was thinking about something along the lines of FileIO in the beginning of its development. Whenever someone tried to load a format with load("file.myformat") the package would figure out if the specific package was installed and would suggest to install it otherwise with a yes/no prompt. The user would just press ENTER and keep going with the original task.

Right now the user has to interrupt his/her flow of thought, jump in the package manager, and then later come back to what he/she was originally doing. Maybe a prompt=true option set by default could help instead of interactive.

[ablaom]

So to flush out @juliohm 's proposal, we have a single new flag prompt=true (default) with this behaviour: If prompt=true then:

• Ask use whether to install package if not already installed. The name of the environment should probably be mentioned in the prompt
• Ask user which package to use if the same model name is duplicated in the registry
• If either of these two scenarios occur, issue a notice that user can set prompt=false to overide interactive behaviour (unless verbosity <1, in which case no notice is given)

If prompt=false then keep current "throw informative error" behaviour.

@juliohm Is this close enough to what you have in mind?

This would be more convenient and beginner-friendly than the status quo. Here are the down-sides as I see them:

• Interactive behaviour by default (might cause issues when used programatically, eg in a package module)
• Mixes package management with package loading. Some might argue package management should be stay user's responsibility (dangers of "secret knowledge", etc).

This small bit of the interface has been a frequent discussion point and changes. Be great to get more feedback before proceeding.

@ExpandingMan @DilumAluthge @tlienart @CameronBieganek Your thoughts?

[DilumAluthge]

• Interactive behaviour by default (might cause issues when used programatically, eg in a package module)

This is a deal-breaker for me.

If we put interactive behavior into the @load macro, it should not be by default. E.g. if we have a kwarg interactive, the default value should be interactive=false.

If we really really need interactive behavior by default, that should go into a separate macro @load_interactive, where the name of this new macro makes it very clear that the behavior will be interactive. So then we can put all of this interactive functionality into the new @load_interactive macro, and we keep @load noninteractive.

[ExpandingMan]

I agree that it would be nice to have some kind of convenience feature for actually installing the packages, especially since the current ecosystem consists of a large number of disparate interface packages. Expecting users to manage these entirely on their own seems excessive, regardless of whether users are "beginners".

That said, I'm not yet sure what I think would be the best approach here. My first thought would be to have a completely separate macro for package installation. I also agree with your concern about the behavior being interactive "by default" (although the default would be prompt=false right?). On one hand, it might be ok because once you've installed a package, it'll appear in the manifest, which means that its installation would be (or could be) fully automated in the future. On the other, I'm concerned that this getting stuck in code without people being abundantly aware that it is potentially interactive (which they surely won't be) unfortunate shenanigans would follow.

So, at first glance, having a keyword argument maybe install which is false by default, which causes it to do Pkg.add if it's not already in the environment seems like a nice feature. I would suggest you think about it a while before implementing though. If I have more thoughts about it in the future, I'll post here.

[codecov-io]

Codecov Report

Merging #346 (cd37ff8) into dev (82e8297) will increase coverage by 0.35%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #346      +/-   ##
==========================================
+ Coverage   79.36%   79.72%   +0.35%     
==========================================
  Files          11       11              
  Lines         848      858      +10     
==========================================
+ Hits          673      684      +11     
+ Misses        175      174       -1     

Impacted Files	Coverage Δ
src/loading.jl	88.65% <100.00%> (+2.45%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82e8297...cd37ff8. Read the comment docs.

[CameronBieganek]

Now that #340 has been merged, maybe the way to go is to make @load default to interactive (prompt=true or interactive=true). The MLJ user manual can recommend that beginners use @load and that developers and more experienced users load models directly by importing from wherever load_path(model) says to import from.

[ablaom]

Thanks for that feedback all. As @ExpandingMan suggests, let's sleep on it.

[ablaom]

Looks good to me.

[ablaom]

Thanks all for the feedback. Although there is no consensus, the best compromise would in my view be:

• merge the current PR as is; and

• in a second PR, add a new method @load_interactive (or @iload??) for interactive loading

@DilumAluthge Are you happy for me to merge?

[DilumAluthge]

Thanks all for the feedback. Although there is no consensus, the best compromise would in my view be:

* merge the current PR as is; and

* in a second PR, add a new method `@load_interactive` (or `@iload`??) for interactive loading

@DilumAluthge Are you happy for me to merge?

Sounds good to me!