-
Notifications
You must be signed in to change notification settings - Fork 101
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
make 'pyinitialize' work under windows #33
Conversation
self.call('pyinitialize(C_NULL)') | ||
if os.name == "nt": | ||
# on windows C_NULL doesn't mean anything | ||
self.call('pyinitialize()') |
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.
@stevengj is this true?
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.
more precisely, I suspect a variable -1 is asked to Py_GetVersion
, which is clueless about its meaning and answers symbol could not be found by py_getversion
I may be wrong on the true cause, but the fix works
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 doesn't seem right to me. Running pyinitialize()
tries to find libpython
by running the python
executable. If you are initializing PyCall from within Python, you want it to use the libpython
that is already loaded into memory.
If you pass C_NULL
to pyinitialize
, it initializes libpython
by special code that loads the base module. That should be the right thing here, I think, in order to use the already-loaded libpython.
C_NULL
is defined by Julia and should be available on all platforms.
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.
Playing this, under IPython:
import julia
j = julia.Julia()
With C_NULL
on winpython, I have this error:
symbol could not be found Py_GetVersion (-1): no error
Without C_NULL
, it works.
Maybe, Error message is more significant under IDLE (I just tried):
>>> import julia
>>> j = julia.Julia()
Traceback (most recent call last):
File "D:\WinPython\basedir34\buildFlavorJuliab4\winpython-3.4.2.amd64\python-3.4.2.amd64\lib\site-packages\julia\core.py", line 294, in __init__
self.call('pyinitialize(C_NULL)')
File "D:\WinPython\basedir34\buildFlavorJuliab4\winpython-3.4.2.amd64\python-3.4.2.amd64\lib\site-packages\julia\core.py", line 336, in call
.format(exception_type, exception_msg, src))
julia.core.JuliaError: Exception 'UndefRefError' ocurred while calling julia code:
<couldn't get stack>
Code:
pyinitialize(C_NULL)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<pyshell#2>", line 1, in <module>
j = julia.Julia()
File "D:\WinPython\basedir34\buildFlavorJuliab4\winpython-3.4.2.amd64\python-3.4.2.amd64\lib\site-packages\julia\core.py", line 298, in __init__
raise JuliaError("Failed to initialize PyCall package")
julia.core.JuliaError: Failed to initialize PyCall package
>>>
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.
@stonebig, the problem here is that using pyinitialize()
is extremely fragile in this context: if it loads a different libpython
than the one that is currently running then everything will break. We need to figure out what the underlying problem is.
The question is, how do we get a handle to the running libpython
library?
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.
So, if my patch allows to have an "extremely fragile" pyjulia on windows, instead of an "already broken" one, shouldn't you call that "progress" and merge it ?
(if I understand your concern, maybe can you create a temporary python variable in the start process of pyjulia and check you can read it via the given handle ? as a poka-yoke)
(maybe also your fear that windows can have 2 different libpython
dll accessible in the same session/process is impossible, and so all is wonderfull with the patch , - + - = +)
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.
I would rather have something that breaks noisily, and hence gets fixed, than something that breaks mysteriously when you do something unrelated on your system (e.g. install another python
executable version in your path, or run python2
when you also have python3
installed).
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.
In particular, it looks like the solution might be to have julia/src/dlload.c
use GetModuleHandle(NULL)
instead of GetModuleHandleExW(...)
in jl_load_dynamic_library_
; this will get the handle of the module for the running executable, which I think is what we need to look up symbols in a globally-loaded libpython
, rather than the libjulia
module handle. (i.e. it seems to be the analogue of the dlopen(NULL)
that jl_load_dynamic_library_
is already doing on Mac and Linux systems).
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.
No idea, sorry. I don't know any better here than to look through those MSDN pages.
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.
On Windows, it looks like GetModuleHandle(NULL)
is already called by Julia and stashed in jl_exe_handle
. I've pushed a patch to PyCall that uses this, and I'm hoping it fixes the problem.
I've just pushed a PyCall patch (JuliaPy/PyCall.jl@b4974b0) that attempts to fix |
Yes, I hope within the coming hour |
ok, I did modify the from within IDLE, result is coming much quicker than before (let say 30" instead of 4') :
Maybe I didn't test right. |
I found back the fibonacci example and checked it works with my initial patch. |
I tested Windows as part of #54 and fixed any issues I encountered. Please try it out and file a new issue if there are any problems. |
No description provided.