-
-
Notifications
You must be signed in to change notification settings - Fork 413
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
Additional workarounds for jupyter-kernelspec.exe #416
Conversation
This proposed change amends and extends the previously included (so-called) hacks for Windows that were added for issue JuliaLang#363 (that are waiting on issue JuliaLang#448 for the jupyter project). These changes allow for IJulia to work a version of Python that has been downloaded from python.org, where jupyter was pip installed with a version recent enough to include jupyter-kernelspec.exe and jupyter-notebook.exe, and where the end user has set the JUPYTER environment variable either at the system level or in ENV within their Julia session prior to building IJulia via Pkg.build. If the issues that led to the discussion in JuliaLang#363 and jupyter issue JuliaLang#448 have been addressed, then the changes in this commit and PR can likely be ignored.
# strip quotes, if any | ||
if python[1] == python[end] == '"' | ||
python = python[2:end-1] | ||
jupyter_dir = splitdir(jupyter)[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dirname(jupyter)
?
Addressing the line notes, JuliaLang#416 (comment) and JuliaLang#416 (comment) added by @stevengj
if python[1] == python[end] == '"' | ||
python = python[2:end-1] | ||
end | ||
elseif isfile(jn_path) || error("$jn_path not found") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't right, because you have to check isfile(jn_path)
after assigning it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True
Updating based on [comment](JuliaLang#416 (comment))
Looks good to merge once the tests are green. |
On AppVeyor, the 32-bit 0.3 test passed, 64-bit 0.3 test seems to have hung/timed-out, and both the 32-bit & 64-bit nightlies tests failed when downloading and installing Conda packages here, which is prior to the location of changes in this PR. Is it possible to re-run the tests without a new commit? |
I restarted the AppVeyor check. |
Bump @stevengj is master here okay to tag? |
Yes, I think we are overdue for a tag. I'm busy today, so feel free to do the honors. |
Thanks, will do. |
This proposed change amends and extends the previously included (so-called) hacks for Windows that were added for issue #363 (that are waiting on issue #448 for the jupyter project). These changes allow for IJulia to work with a version of Python that has been downloaded from python.org, where jupyter was pip installed with a version recent enough to include jupyter-kernelspec.exe and jupyter-notebook.exe, and where the end user has set the JUPYTER environment variable either at the system level or in ENV within their Julia session prior to building IJulia via Pkg.build.
If the issues that led to the discussion in #363 and jupyter issue #448 have been addressed, then the changes in this commit and PR can likely be ignored.