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

refactor py_str macro to allow passing custom global/locals #777

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/pyeval.jl
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@ function pyeval(s::AbstractString, returntype::TypeTuple=PyAny,
return convert(returntype, pyeval_(s, pynamespace(Main), locals, input_type))
end

# get filename from @__FILE__ macro, which returns nothing in the REPL
make_fname(fname::AbstractString) = String(fname)
make_fname(fname::Any) = "REPL"

# a little finite-state-machine dictionary to keep track of where
# we are in Python code, since $ is ignored in string literals and comments.
# 'p' = Python code, '#' = comment, '$' = Julia interpolation
Expand Down Expand Up @@ -202,32 +198,37 @@ pasted into the Python code. This allows you to evaluate code
where the code itself is generated by a Julia expression.
"""
macro py_str(code, options...)
m = :(pynamespace($__module__))
fname = String(__source__.file)
esc(:(PyCall.@_py_str($m, $m, $fname, $code, $(options...))))
Copy link
Member

Choose a reason for hiding this comment

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

Why is this call to esc necessary, since there are esc calls within @_py_str?

Indeed, the call to esc here seems like it might cause problems, because it prevents the names pylocals, pyglobals, and ret from being hygienized.

Copy link
Contributor Author

@marius311 marius311 May 15, 2020

Choose a reason for hiding this comment

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

No, pylocals etc... get hygenized by @_py_str, the esc here makes it so the interpolated variables don't also get hygenized, e.g. with the code in this PR:

julia> @macroexpand py"1+$i"
quote
    #= /home/marius/src/pyjulia/dev/PyCall/src/pyeval.jl:229 =#
    (var"#18#pyglobals", var"#19#pylocals") = (PyCall.pynamespace(Main), PyCall.pynamespace(Main))
    #= /home/marius/src/pyjulia/dev/PyCall/src/pyeval.jl:230 =#
    begin
        var"#19#pylocals"["__julia_localvar_2_1"] = PyCall.PyObject(i)
    end
    #= /home/marius/src/pyjulia/dev/PyCall/src/pyeval.jl:231 =#
    var"#20#ret" = (PyAny)(PyCall.pyeval_((string)("1+__julia_localvar_2_1"), var"#18#pyglobals", var"#19#pylocals", 258, "none"))
    #= /home/marius/src/pyjulia/dev/PyCall/src/pyeval.jl:232 =#
    begin
        PyCall.delete!(var"#19#pylocals", "__julia_localvar_2_1")
    end
    #= /home/marius/src/pyjulia/dev/PyCall/src/pyeval.jl:233 =#
    var"#20#ret"
end

Copy link
Member

Choose a reason for hiding this comment

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

Maybe I should've asked this before but why is insert_pyevals run at run-time? It looks like this doesn't depend on the run-time value of globals and locals (or maybe I'm missing something)? If we expand everything at macro-expansion time, perhaps esc of @py_str here and @prepare_for_pyjulia_call there can be removed? Then maybe _py_str doesn't have to be a macro?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
esc(:(PyCall.@_py_str($m, $m, $fname, $code, $(options...))))
esc(:($PyCall.@_py_str($m, $m, $fname, $code, $(options...))))

I think you need this?

I think we better test py"" with something like

baremodule TestPyStr
using PyCall: @py_str
pystr(x) = py"$$x"
end
@test TestPyStr.pystr("1+1") == 2

end

macro _py_str(pyglobals, pylocals, fname, code, options...)
T = length(options) == 1 && 'o' in options[1] ? PyObject : PyAny
code, locals = interpolate_pycode(code)
input_type = '\n' in code ? Py_file_input : Py_eval_input
fname = make_fname(@__FILE__)
assignlocals = Expr(:block, [(isa(v,String) ?
:(m[$v] = PyObject($(esc(ex)))) :
:(pylocals[$v] = PyObject($(esc(ex)))) :
nothing) for (v,ex) in locals]...)
code_expr = Expr(:call, esc(:(Base.string)))
code_expr = Expr(:call, Base.string)
i0 = firstindex(code)
for i in sort!(collect(filter(k -> isa(k,Integer), keys(locals))))
push!(code_expr.args, code[i0:prevind(code,i)], esc(locals[i]))
i0 = i
end
push!(code_expr.args, code[i0:lastindex(code)])
if input_type == Py_eval_input
removelocals = Expr(:block, [:(delete!(m, $v)) for v in keys(locals)]...)
removelocals = Expr(:block, [:(delete!(pylocals, $v)) for v in keys(locals)]...)
else
# if we are evaluating multi-line input, then it is not
# safe to remove the local variables, because they might be referred
# to in Python function definitions etc. that will be called later.
removelocals = nothing
end
quote
m = pynamespace($__module__)
pyglobals, pylocals = $pyglobals, $pylocals
$assignlocals
ret = $T(pyeval_($code_expr, m, m, $input_type, $fname))
ret = $T(pyeval_($code_expr, pyglobals, pylocals, $input_type, $fname))
$removelocals
ret
end
Expand Down