Use JULIA_PROJECT env variable to activate julia env #612
Conversation
| @@ -118,7 +122,7 @@ def get_assemble_scripts(self): | |||
| ( | |||
| "${NB_USER}", | |||
| r""" | |||
| julia -e "using Pkg; Pkg.add(\"IJulia\"); using IJulia; installkernel(\"Julia\", \"--project=${REPO_DIR}\", env=Dict(\"JUPYTER_DATA_DIR\"=>\"${NB_PYTHON_PREFIX}/share/jupyter\"));" && \ | |||
| JULIA_PROJECT="" julia -e "using Pkg; Pkg.add(\"IJulia\"); using IJulia; installkernel(\"Julia\", env=Dict(\"JUPYTER_DATA_DIR\"=>\"${NB_PYTHON_PREFIX}/share/jupyter\"));" && \ | |||
There was a problem hiding this comment.
For my education: Why set it to an empty value here? Not sure what I was expecting but explicitly setting it to be empty surprised me :)
There was a problem hiding this comment.
I assume this is to make sure we install the default version of IJulia? But actually, that is a bit interesting... might we not also want to allow the user to specify a different version of IJulia if they so choose?
EDIT: after thinking more, this is probably actually so that we don't modify the user's environment, potentially breaking some reproducibility. We don't want to install anything into their environment, but we do need to install IJulia, and so we install it into the default environment. Since this PR is setting the project globally through the environment variable, we need to disable the setting so that julia will open w/ the default environment.
There was a problem hiding this comment.
I wonder if we should activate and instantiate the user's project first, and then only add IJulia to the default environment if they don't already have IJulia installed? And only then install the kernel?
(Note that my above comments are orthogonal to this PR)
There was a problem hiding this comment.
Yes, @NHDaly is correct: By setting this to "" we make sure the IJulia package gets installed into the default environment, not the user specified environment. Later, these environments are automatically stacked by julia. So if a user also had an IJulia version in their Project.toml, it would take precedence over the one we installed in the default environment.
I don't think we should install IJulia into the user provided env: that can potentially lead to updates of over, unrelated packages, and then we really break reproducability.
There was a problem hiding this comment.
I don't think we should install IJulia into the user provided env: that can potentially lead to updates of over, unrelated packages, and then we really break reproducability.
Yeah, I had the same concern. I agree with you here 100%!
So if a user also had an IJulia version in their Project.toml, it would take precedence over the one we installed in the default environment.
But what i was concerned about is that if a user specifies a different version of IJulia in their Project.toml, we're still going to install the kernel from this default version, not the one they specify. Would it be crazy to peek ahead and only install IJulia into the default env if the user doesn't include it in theirs? (Either way, this seems like a feature that could be added in a later PR)
|
I think my only question (as a none Julia user) is: In what ways could we be breaking users with this change? Should we add a FAQ entry to help them or even define both |
|
Hmm, i'm not sure if this makes sense. And in fact, I actually didn't notice that you added this On the one hand, I like the spirit of it: Yeah, you're using the Project.toml to specify what your environment should look like, so yeah you'd want that environment to be activated when you run notebooks! It will be nice not to have to run But it is a bit unexpected to me because it's not how the default IJulia kernel behaves. So if I clone this repo and run jupyter from there locally, my notebook won't work, even though the repo has the Project.toml file in it... I would need to add I do agree that it would be nice to avoid the boilerplate That said, I think I share your feeling that setting this environment variable is better than setting that flag in the Jupyter Kernel, but I'm not very sure. I don't think we'd want the shell to open into the default environment, since presumably users would be using the shell to modify things about the notebooks. One case that may be harmed would be if they wanted to make changes to the IJulia installation itself; it's difficult to reactivate the default environment once you've opened Julia with this flag set, and people may not know why that's happening, since I don't think this flag is commonly used. I could imagine people typing |
|
(After some investigation, i've just discovered that you can reactivate the default environment via |
I think none, given that this only affects the
Yes, that is a good point. Lets see whether we can change the IJulia default: JuliaLang/IJulia.jl#820. I think at this point the main question is whether we want to activate the env in the So I think maybe we can merge this here, but then also keep the option open to actually not activate the environment for the notebook at all, depending on how the discussion over in the IJulia issue turns out? I guess another question is how this works for other languages in binder? I assume that if there is a requirements.txt file in the root, and then you run a notebook in that repo, the packages from that are just available, and you don't have to somehow activate things in some way? |
Yes! I love the idea of doing this by default in other kernels as well. Great idea, @davidanthoff! :) +1
The only potential downside I could see is that this PR enables the same behavior for julia shells opened in the terminal, which is also a bit unexpected and different from the normal One more question: is |
|
So an update on this one: IJulia now merged this behavior, so for Jupyter we should be in the clear with auto activating the environment. So the question now is whether we also want to auto-activate the environment for any other julia process, i.e. a non-IJulia kernel, or not. I think we should, actually :) Isn't the whole point of binder that you get a pre-populated environment? One question that might be good to sort out is how users can actually start a julia process other than an IJulia kernel? I'm just not familiar enough with the binder setup, maybe @betatim can shed some light on this? Can one e.g. SSH into a binder instance? |
|
This might be a silly question, but now that IJulia has merged this behavior, can we just rely on IJulia to do it by default, instead of implementing it ourselves?
One way that I can think of: And it looks like this: So the question is, if we open the Julia REPL through that terminal, should it activate your environment? My gut is to vote No, since that's not how the terminal behaves on your local computer. And anyone who's managed to put a Project.toml file in their repo knows how to activate a directory. But I do also see how it makes sense from a usability perspective: why would you ever NOT want your project activated? Also, since this docker instance is short lived, and about to be killed, I can't see any harm in having your project activated when you didn't expect it. You're not going to break anything! SO I guess what i'm saying is yeah I think your change makes sense, @davidanthoff. :) I've talked myself into it, and now I vote Yes! |
|
I think we should now first merge #622, and then I can resolve any conflicts between that and this PR here, and then we can merge this PR here. |
davidanthoff
changed the title
davidanthoff
changed the title
|
This is ready from my end. @NHDaly do you want to review this final version? I also removed |
|
Great, thanks! @betatim ready for your review/merge! |
|
Thanks! LGTM -> merged. |
|
Great, thanks! |
…handling Use JULIA_PROJECT env variable to activate julia env
Fixes #610.
This should be much more robust, plus is now also means that if one starts a julia at the terminal or something like that, all the packages from the environment are also available.
@NHDaly do you think this makes sense?