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

Document that the REPL workflow assumes your module is in LOAD_PATH #24223

Closed
wants to merge 5 commits into from

Conversation

rdeits
Copy link
Contributor

@rdeits rdeits commented Oct 19, 2017

Inspired by https://discourse.julialang.org/t/repl-workflow-reload-doesnt-work/6571 I noticed that the workflow tips section of the manual quietly assumes that the module the user is developing is on the LOAD_PATH. I've added a bullet point to make that requirement explicit.

Please let me know if the wording of this is OK.

@Wikunia
Copy link
Contributor

Wikunia commented Oct 19, 2017

Thanks @rdeits You might want to include push!(LOAD_PATH, pwd()) which you mentioned in the forum.

@rdeits
Copy link
Contributor Author

rdeits commented Oct 19, 2017

I originally wrote that, but I wasn't sure if that was the best thing to recommend in the manual. If a user does push!(LOAD_PATH, pwd()) at the REPL, that's likely to be fine, but if they put it in a script, then their code will break if the script is run as julia subfolder/foo.jl instead of julia foo.jl. In a script, it would be better to do push!(LOAD_PATH, @__DIR__).

In Julia v0.7, @__DIR__ gives the current working directory when run from the REPL, but in v0.6 it just gives nothing. So I think that push!(LOAD_PATH, @__DIR__) is the right thing to recommend in all cases on v0.7, but won't work on v0.6.

@rdeits
Copy link
Contributor Author

rdeits commented Oct 19, 2017

Although I guess this change will only affect the v0.7 manual, right? So then perhaps the most correct thing would be to change the example to:

push!(LOAD_PATH, @__DIR__)

@Wikunia
Copy link
Contributor

Wikunia commented Oct 20, 2017

Btw I thing the error message Module Tmp not found in current path. is quite misleading. The module is actually in the current path, right? It isn't in the LOAD_PATH but for me current means the cwd (current working directory).

@rdeits
Copy link
Contributor Author

rdeits commented Oct 20, 2017

I totally agree. Would you like me to come up with a better wording for it in this PR, or should we save that for the next one?

@Wikunia
Copy link
Contributor

Wikunia commented Oct 20, 2017

I don't mind. I think I would replace current path with LOAD_PATH. I think that makes it pretty straightforward. If someone doesn't know what that is he can google and probably find the solution quickly.

@rdeits
Copy link
Contributor Author

rdeits commented Oct 20, 2017

How about "LOAD_PATH or Pkg.dir()", since it could be in either location?

@Wikunia
Copy link
Contributor

Wikunia commented Oct 20, 2017

Yes that's perfect! Thank you!

@fredrikekre fredrikekre added the docs This change adds or pertains to documentation label Oct 22, 2017
@rdeits
Copy link
Contributor Author

rdeits commented Oct 23, 2017

Ok, I've made the change to "LOAD_PATH or Pkg.dir()"

@rdeits
Copy link
Contributor Author

rdeits commented Oct 30, 2017

Hm, it looks like having nothing in the LOAD_PATH is actually a problem on v0.6:

julia> push!(LOAD_PATH, @__DIR__)
3-element Array{Any,1}:
 "/Applications/Julia-0.6.app/Contents/Resources/julia/local/share/julia/site/v0.6"
 "/Applications/Julia-0.6.app/Contents/Resources/julia/share/julia/site/v0.6"
 nothing

julia> using StaticArrays
ERROR: ArgumentError: unrecognized custom loader in LOAD_PATH: nothing
Stacktrace:
 [1] load_hook(::Void, ::String, ::String) at ./loading.jl:94
 [2] find_in_path(::String, ::Void) at ./loading.jl:116
 [3] find_in_node_path(::String, ::Void, ::Int64) at ./loading.jl:125
 [4] _require(::Symbol) at ./loading.jl:426
 [5] require(::Symbol) at ./loading.jl:398

julia> versioninfo()
Julia Version 0.6.0
Commit 903644385b (2017-06-19 13:05 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin13.4.0)
  CPU: Intel(R) Core(TM) i7-2860QM CPU @ 2.50GHz
  WORD_SIZE: 64
  BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Sandybridge)
  LAPACK: libopenblas64_
  LIBM: libopenlibm
  LLVM: libLLVM-3.9.1 (ORCJIT, sandybridge)

but everything is fine on v0.7. Is that still ok, since this is just a change to the 0.7+ manual?

@StefanKarpinski
Copy link
Member

What is having nothing on LOAD_PATH supposed to do?

@rdeits
Copy link
Contributor Author

rdeits commented Oct 30, 2017

It's not supposed to do anything. This came up because @__DIR__ in v0.7 returns the current directory, but in v0.6 returns nothing. So the recommendation that I'm adding to the manual (push!(LOAD_PATH, @__DIR__)) works fine on v0.7 but will cause the above error if people try to do it on v0.6. I'm just wondering if that's okay.

@fredrikekre
Copy link
Member

This change won't end up in the v0.6 docs anyway, so should be ok.

@rdeits
Copy link
Contributor Author

rdeits commented Oct 31, 2017

Ok, great!

@rdeits
Copy link
Contributor Author

rdeits commented Nov 12, 2017

Is this good to go?

@vtjnash
Copy link
Member

vtjnash commented May 8, 2018

supplanted by #26804?

@rdeits
Copy link
Contributor Author

rdeits commented May 8, 2018

I think the workflow-tips change would still be beneficial, but I'll see how it shakes out with the new package manager. Anyway, yeah, this seems to have gone stale.

@rdeits rdeits closed this May 8, 2018
@dstndstn
Copy link

On my vanilla OSX setup and Julia 1.0, the Workflow Tips section of the manual does not work as advertised; this PR would help immensely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants