JuliaInterop / NBInclude.jl

import code from IJulia Jupyter notebooks into Julia programs
Other
118 stars 17 forks source link

Consistency with IJulia and NBInclude with the soft global scope? #11

Closed jlperla closed 6 years ago

jlperla commented 6 years ago

First of all, I love the new soft global scope. But is NBInclude.jl using it the same was as IJulia?

Here is my test case. Create a cell with

n = 4
for i=1:2
    @show i
    n+= i
end
n

Then another, separate, code cell beneath it with just

n

If you run the two cells in Jupyter, it works great. No globals required, and the second n correctly captures the modified value, as you need for any notebook style programming.

But if I run that same notebook with NBInclude.jl it says

julia> @nbinclude("Untitled.ipynb")

ERROR: LoadError: UndefVarError: n not defined
Stacktrace:
 [1] top-level scope at /mnt/c/Users/jlperla/Documents/projects/Untitled.ipynb:In[4]:3 [inlined]
 [2] top-level scope at ./none:0
 [3] include_string(::Module, ::String, ::String) at ./loading.jl:1002
 [4] my_include_string(::Module, ::String, ::String, ::Nothing) at /home/jlperla/.julia/packages/NBInclude/YVOMm/src/NBInclude.jl:31
 [5] #nbinclude#1(::Bool, ::UnitRange{Int64}, ::Regex, ::typeof(identity), ::Function, ::Module, ::String) at /home/jlperla/.julia/packages/NBInclude/YVOMm/src/NBInclude.jl:83
 [6] nbinclude(::Module, ::String) at /home/jlperla/.julia/packages/NBInclude/YVOMm/src/NBInclude.jl:54
 [7] top-level scope at none:0
in expression starting at /mnt/c/Users/jlperla/Documents/projects/Untitled.ipynb:In[4]:2

For my packages

(v1.0) pkg> st
    Status `~/.julia/environments/v1.0/Project.toml`
  [c52e3926] Atom v0.7.4
  [0c46a032] DifferentialEquations v5.3.0
  [31c24e10] Distributions v0.16.2
  [2fe49d83] Expectations v1.0.1
  [7073ff75] IJulia v1.10.0
  [a98d9a8b] Interpolations v0.8.0
  [0db19996] NBInclude v2.0.1
  [429524aa] Optim v0.17.0
  [d96e819e] Parameters v0.9.2
  [91a5bcdd] Plots v0.20.1
  [295af30f] Revise v0.7.3

(I tried to upload the notebook, but github won't let me)

stevengj commented 6 years ago

My feeling was that nbinclude should continue to work like include — in particular, once you start re-using code in files, you should start using global declarations.

It's not an ideal situation, but that seems to be the only non-breaking resolution to https://github.com/JuliaLang/julia/issues/28789 — only use soft global scope for "interactive" code.

jlperla commented 6 years ago

I see. Is there a way to make it an option, or perhaps some code to manually call the soft global around the @nbinclude call to emulate it?

The issue is you sometimes (e.g. the unit testing of notebooks we want to use it with) have the code run exactly as it would were it in jupyter.

jlperla commented 6 years ago

@stevengj I tried to wrap the @nbinclude in a call to SoftGlobalScope.jl but was not successful. My attempt was to add in the example from above in a notebook called global_test.ipynb and then in the REPL, something like

using SoftGlobalScope, NBInclude
ex = softscope(Main, :(@nbinclude("global_test.ipynb")))
eval(ex)

... but that didn't work, and it seemed to give the same scoping "error" as if I was running it in the REPL. Am I using it incorrectly? Is this possible to hack?

stevengj commented 6 years ago

I don't think there's a good way to do it without editing NBInclude.jl itself.

I think the right approach is to make it an option of nbinclude.

jlperla commented 6 years ago

Gotcha. Is that an easy or tricky change to make... and is it better to let the dust settle with the REPL and soft-global first?

stevengj commented 6 years ago

Easy change.

stevengj commented 6 years ago

In my_include_string, just change include_string(m, s, path) to softscope_include_string(m, s, path). A PR doing this (based on the value of a softscope::Bool=false keyword argument to nbinclude) would be welcome.

jlperla commented 6 years ago

@arnavs Can you give this a shot, and add a test case with the global which breaks vs doesn't break depending on the toggle?

arnavs commented 6 years ago

@jlperla Sure thing.

arnavs commented 6 years ago

OK, I think this is done.

julia> @nbinclude("scoping.ipynb")
ERROR: LoadError: UndefVarError: a not defined
Stacktrace:
 [1] top-level scope at /Users/arnavsood/.julia/dev/NBInclude/test/includes/scoping.ipynb:In[2]:4 [inlined]
 [2] top-level scope at ./none:0
 [3] include_string(::Module, ::String, ::String) at ./loading.jl:1002
 [4] my_include_string(::Module, ::String, ::String, ::Nothing, ::Bool) at /Users/arnavsood/.julia/dev/NBInclude/src/NBInclude.jl:31
 [5] #nbinclude#1(::Bool, ::UnitRange{Int64}, ::Regex, ::typeof(identity), ::Bool, ::Function, ::Module, ::String) at /Users/arnavsood/.julia/dev/NBInclude/src/NBInclude.jl:84
 [6] nbinclude(::Module, ::String) at /Users/arnavsood/.julia/dev/NBInclude/src/NBInclude.jl:55
 [7] top-level scope at none:0
in expression starting at /Users/arnavsood/.julia/dev/NBInclude/test/includes/scoping.ipynb:In[2]:3

julia> @nbinclude("scoping.ipynb"; softscope=true)
11

Will submit as a PR.