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

allow nested variable interpolation #381

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

marius311
Copy link
Contributor

When I wrote #262, it relied on taking py"..." expressions inside of %julia blocks and rewriting them to some custom code via make_pyeval which was basically a poorman's version of PyCall.@py_str (the code way basically copied from there in fact). The reason for this is because PyCall.@py_str didn't allow passing in a custom globals/locals dict, as is necessary for IPython magic.

In JuliaPy/PyCall.jl#777, I've now made a way that you can pass these globals/locals (via a helper PyCall.@_py_str), so this PR removes my poorman's version and calls PyCall directly, so these py"..."s aren't treated any differently than outside of %julia blocks.

Notably, and the reason for doing this, is that before you couldn't interpolate into py"..."s inside %julia blocks, whereas now you can:


In [2]: i = 1                                                                                                                       

In [3]: %%julia 
   ...: i = $i 
   ...: py"$i" 

Out[3]: 1

This has come up a couple of times in my heavily interop'ed code and is nice to have, plus the lack of code duplication across pyjulia/PyCall is appealing.

What I don't know how best to do, and could use some guidance @tkf @stevengj is how best to make this backwards compatible here. Should I just have an if PyCall.version < XXX and then basically paste the entire contents of the new PyCall.@_py_str, then just drop it in a (very) future version?

Happy to take any other comments on this as well.

@tkf
Copy link
Member

tkf commented May 12, 2020

Notably, and the reason for doing this, is that before you couldn't interpolate into py"..."s inside %julia blocks, whereas now you can

Sounds great. Thanks for fixing this.

Should I just have an if PyCall.version < XXX and then basically paste the entire contents of the new PyCall.@_py_str, then just drop it in a (very) future version?

Yeah, this seems to be the safest option. We can't control PyCall version as it might be in the user's Manifest file.

Once we decide the API on the PyCall side and a few tests, I think it's good to go. As for the test, I think we need one that exercises the vendored version of @_py_str (or whatever the API would be). I think it'd be something like src/julia/tests/test_sysimage.py is doing.

@marius311
Copy link
Contributor Author

Ok, so if you guys have no objections to it being a macro (see other PR), all I need to do then is add the backward compatibility and tests, which I'll work on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants