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

Julia 1.0 support #183

Merged
merged 11 commits into from
Aug 22, 2018
8 changes: 4 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ python:
env:
matrix:
- JULIA_VERSION=0.6.4 CROSS_VERSION=1
- JULIA_VERSION=0.7.0-rc2
- JULIA_VERSION=1.0.0
# - JULIA_VERSION=nightly
global:
- TOXENV=py
Expand All @@ -17,7 +17,7 @@ matrix:
- language: generic
env:
- PYTHON=python2
- JULIA_VERSION=0.7.0-rc2
- JULIA_VERSION=1.0.0
# - JULIA_VERSION=nightly
os: osx
- language: generic
Expand All @@ -29,7 +29,7 @@ matrix:
- language: generic
env:
- PYTHON=python3
- JULIA_VERSION=0.7.0-rc2
- JULIA_VERSION=1.0.0
# - JULIA_VERSION=nightly
os: osx
- language: generic
Expand All @@ -54,7 +54,7 @@ before_script:
- which $PYTHON
- $PYTHON -m pip --version
- $PYTHON -m pip install --quiet tox
- julia -e 'Pkg.add("PyCall")'
- julia --color=yes -e 'VERSION >= v"0.7.0-DEV.5183" && using Pkg; Pkg.add("PyCall")'
script:

# "py,py27" below would be redundant when the main interpreter is
Expand Down
10 changes: 9 additions & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,15 @@ build_script:
# - C:\projects\julia\bin\julia -e "versioninfo(); Pkg.add(\"PyCall\"); Pkg.init(); Pkg.resolve()"
# - C:\projects\julia\bin\julia -e "using PyCall; @assert isdefined(:PyCall); @assert typeof(PyCall) === Module"
- "SET PYTHON=%PYTHONDIR%\\python.exe"
- C:\projects\julia\bin\julia -e "versioninfo(); Pkg.add(\"PyCall\")"
- C:\projects\julia\bin\julia -e "
if VERSION >= v\"0.7.0-DEV.3630\";
using InteractiveUtils;
versioninfo(verbose=true);
else
versioninfo(true);
end;
VERSION >= v\"0.7.0-DEV.5183\" && using Pkg;
Pkg.add(\"PyCall\")"
- "%PYTHONDIR%\\python.exe -m pip install --quiet tox"

test_script:
Expand Down
19 changes: 13 additions & 6 deletions julia/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,14 +262,21 @@ def juliainfo(runtime='julia'):
[runtime, "-e",
"""
println(VERSION < v"0.7.0-DEV.3073" ? JULIA_HOME : Base.Sys.BINDIR)
if VERSION >= v"0.7.0-DEV.3630"
using Libdl
using Pkg
end
println(Libdl.dlpath(string("lib", splitext(Base.julia_exename())[1])))
println(unsafe_string(Base.JLOptions().image_file))
PyCall_depsfile = Pkg.dir("PyCall","deps","deps.jl")
if VERSION < v"0.7.0"
PyCall_depsfile = Pkg.dir("PyCall","deps","deps.jl")
else
modpath = Base.locate_package(Base.identify_package("PyCall"))
PyCall_depsfile = joinpath(dirname(modpath),"..","deps","deps.jl")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines above are for suppressing "Pkg.dir(pkgname, paths...) is deprecated" warning.

I took the code from Pkg.dir: https://github.com/JuliaLang/Pkg.jl/blob/df546844d98dfe233889e91a86f076e01926f69e/src/API.jl#L456-L463 (Note: Base.locate_package(::Nothing) = nothing is defined so I don't think we need to guard against Base.identify_package returning nothing as Pkg.dir does.)

end
if isfile(PyCall_depsfile)
eval(Module(:__anon__),
Expr(:toplevel,
:(Main.Base.include($PyCall_depsfile)),
:(println(pyprogramname))))
include(PyCall_depsfile)
println(pyprogramname)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the reason for putting this in an anonymous module was to avoid putting pyprogramname into the global namespace. Is this no longer a concern?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was Julia 0.4 compatibility or something. This is executed in a subprocess which exits right after printing pyprogramname. So I don't think we have namespace problem. It is added in fde40d7#diff-2f4f81856f2b92c6a4bb1e5bba2cf28aR260

end
"""])
args = output.decode("utf-8").rstrip().split("\n")
Expand Down Expand Up @@ -342,7 +349,7 @@ def __init__(self, init_julia=True, jl_runtime_path=None, jl_init_path=None,
runtime = jl_runtime_path
else:
runtime = 'julia'
JULIA_HOME, libjulia_path, image_file, depsjlexe = juliainfo()
JULIA_HOME, libjulia_path, image_file, depsjlexe = juliainfo(runtime)
self._debug("pyprogramname =", depsjlexe)
self._debug("sys.executable =", sys.executable)
exe_differs = is_different_exe(depsjlexe, sys.executable)
Expand Down
32 changes: 30 additions & 2 deletions julia/with_rebuilt.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from __future__ import print_function, absolute_import

import os
import signal
import subprocess
import sys
from contextlib import contextmanager
Expand All @@ -21,14 +22,20 @@ def maybe_rebuild(rebuild, julia):
env = os.environ.copy()
info = juliainfo(julia)

build = [julia, '-e', 'Pkg.build("PyCall")']
build = [julia, '-e', """
if VERSION >= v"0.7.0-DEV.3630"
using Pkg
end
Pkg.build("PyCall")
"""]
print('Building PyCall.jl with PYTHON =', sys.executable)
print(*build)
sys.stdout.flush()
subprocess.check_call(build, env=dict(env, PYTHON=sys.executable))
try:
yield
finally:
print() # clear out messages from py.test
print('Restoring previous PyCall.jl build...')
print(*build)
if info.pyprogramname:
Expand All @@ -42,8 +49,29 @@ def maybe_rebuild(rebuild, julia):
yield


@contextmanager
def ignoring(sig):
"""
Context manager for ignoring signal `sig`.

For example,::

with ignoring(signal.SIGINT):
do_something()

would ignore user's ctrl-c during ``do_something()``. This is
useful when launching interactive program (in which ctrl-c is a
valid keybinding) from Python.
"""
s = signal.signal(sig, signal.SIG_IGN)
try:
yield
finally:
signal.signal(sig, s)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a comment about what's going on here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a docstring.



def with_rebuilt(rebuild, julia, command):
with maybe_rebuild(rebuild, julia):
with maybe_rebuild(rebuild, julia), ignoring(signal.SIGINT):
print('Execute:', *command)
return subprocess.call(command)

Expand Down
12 changes: 9 additions & 3 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import sys
import os

import pytest

python_version = sys.version_info


Expand Down Expand Up @@ -56,7 +58,7 @@ def add(a, b):
self.assertTrue(all(x == y for x, y in zip([11, 11, 11],
julia.map(lambda x: x + 1,
array.array('I', [10, 10, 10])))))
self.assertEqual(6, julia.foldr(add, 0, [1, 2, 3]))
self.assertEqual(6, julia.reduce(add, [1, 2, 3]))

def test_call_python_with_julia_args(self):
self.assertEqual(6, sum(julia.eval('(1, 2, 3)')))
Expand Down Expand Up @@ -97,8 +99,8 @@ def test_from_import_non_existing_julia_name(self):
def test_julia_module_bang(self):
from julia import Base
xs = [1, 2, 3]
ys = Base.scale_b(xs[:], 2)
assert all(x * 2 == y for x, y in zip(xs, ys))
ys = Base.fill_b(xs[:], 10)
Copy link
Member

@stevengj stevengj Aug 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not ys = Base.fill_b(xs, 10) ?

But I would just set ys = [2, 4, 6] and keep the old test all(x * 2 == y for x, y in zip(xs, ys))

If we want to test the "bang" in-place operators we should do so with a numpy array where we can check that it is actually operating in-place.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old test does not work since scale! is gone in Julia 1.0. The easiest bang method that I could find both in Julia 0.6 and 1.0 was fill!. And indeed xs[:] is redundant. Thanks for catching thins.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize that, but the new test doesn't actually test much. That's why I suggested just setting ys = [2, 4, 6] and keeping the test of all as-is.

Copy link
Member Author

@tkf tkf Aug 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention when writing the test was just to test variable renaming code path (back then I wasn't sure if it was OK to import numpy). But now that we have it via tox, I suggest something like:

xs = numpy.zeros(3)
Base.fill_b(xs, 10)
assert all(ys == 10)

or

xs = numpy.array([1, 2, 3])
ys = xs.copy()
if julia.eval('VERSION >= v"0.7-"'):
    Base.rmul_b(xs, 2)
else:
    Base.scale_b(xs, 2)
assert all(2 * ys == xs)

Which one do you like?

Edit: ys = xs[:] -> ys = xs.copy(). Coding to much in Julia. I start forgetting Python :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the test I proposed does not work. Strange...

    def test_julia_module_bang(self):
        from julia import Base
        xs = numpy.zeros(3)
        ys = Base.fill_b(xs, 10)
        assert all(ys == 10)
>       assert all(xs == 10)
E       AssertionError: assert False
E        +  where False = all(array([0., 0., 0.]) == 10)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that was because py"xs" was converted to PyAny. If I do fill!(PyArray(PyCall.maindict()["xs"]), 10) in Julia, mutation works (= it's reflected on Python side).

I'm not sure what's the right illustration of this problem in Julia. Something like this?:

julia> py"""
       def apply(f):
           xs = numpy.zeros(3)
           f(xs)
           return xs
       """

julia> py"apply"(xs -> fill!(xs, 10))
3-element Array{Float64,1}:
 0.0
 0.0
 0.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See JuliaPy/PyCall.jl#531 (comment) … you need to specify the type conversion you want for an argument if you want in-place operations to be possible in general.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Yes, I think explicitly denoting which argument is mutating is a good idea.

So, for this test, we just need to check the returned value, right?:

    def test_julia_module_bang(self):
        from julia import Base
        xs = [1, 2, 3]
        ys = Base.fill_b(xs, 10)
        assert all(y == 10 for y in ys)

Although it may be better if we just drop _b since users have to implement a callback on Julia side to avoid allocations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. So I think I finally found a meaningful test:

    def test_julia_module_bang(self):
        from julia.Base import Channel, put_b, take_b
        chan = Channel(1)
        sent = 123
        put_b(chan, sent)
        received = take_b(chan)
        assert sent == received

What do you think?

In the future, I think we need an API to denote some Python values have to be passed without copying. Something like fill_b(asref(arr), 10).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the right thing to do is to make it easy to use the pyfunction method from Julia, for cases where you need control over precisely how arguments are passed.

assert all(y == 10 for y in ys)

def test_import_julia_submodule(self):
from julia.Base import Enums
Expand All @@ -121,6 +123,10 @@ def test_module_dir(self):
from julia import Base
assert 'resize_b' in dir(Base)

@pytest.mark.skipif(
"JULIA_EXE" in orig_env,
reason=("cannot be tested with custom Julia executable;"
" JULIA_EXE is set to {}".format(orig_env.get("JULIA_EXE"))))
def test_import_without_setup(self):
command = [sys.executable, '-c', 'from julia import Base']
print('Executing:', *command)
Expand Down