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

Support virtual environment #578

Merged
merged 29 commits into from Feb 6, 2019
Merged

Support virtual environment #578

merged 29 commits into from Feb 6, 2019

Conversation

tkf
Copy link
Member

@tkf tkf commented Sep 21, 2018

This PR grew out of a small feature addition to a patch that includes a few more. Here is a new summary:

  • Main feature addition: Control Python environment at run-time by the environment variables PYCALL_JL_RUNTIME_PYTHON (and PYCALL_JL_RUNTIME_PYTHONHOME).
  • fix ImportError: No module named 'encodings' when using with virtualenv + python3 #410: Support building Pycall against a Python executable generated by python -m venv.
  • Refactoring: I merged how Py_SetPythonHome and Py_SetProgramName are ccalled with and without PYCALL_JL_RUNTIME_PYTHON. wpyprogramname and wPYTHONHOME are removed.

Original summary:

I suggest to let users control Python environment at runtime by (say) ENV["PYCALL_JL_RUNTIME_PYTHON"] (and ENV["PYCALL_JL_RUNTIME_PYTHONHOME"]). This can be used to make sure that Julia and Python environments for a project are fully reproducible. Combining Pkg3.jl and pipenv, a typical usage would be very simple:

$ ls
activate.jl  Manifest.toml  Pipfile  Pipfile.lock  Project.toml
$ cat activate.jl
import Pkg
Pkg.activate(@__DIR__)
ENV["PYCALL_JL_RUNTIME_PYTHON"] = strip(read(setenv(`pipenv --py`, dir=@__DIR__), String))
$ julia -i activate.jl
julia> using PyCall
julia> pyimport("numpy")  # loads numpy specified by Pipfile

Since this is useful outside PyCall testing, I included this
functionality in python_cmd function instead of inlining it to the
test.
@tkf
Copy link
Member Author

tkf commented Oct 21, 2018

I added some tests and they all pass on CIs now. I think it's good to go. (But please review.)

@codecov-io
Copy link

codecov-io commented Oct 21, 2018

Codecov Report

Merging #578 into master will decrease coverage by 0.65%.
The diff coverage is 30.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #578      +/-   ##
==========================================
- Coverage   53.11%   52.45%   -0.66%     
==========================================
  Files          19       19              
  Lines        1525     1567      +42     
==========================================
+ Hits          810      822      +12     
- Misses        715      745      +30
Impacted Files Coverage Δ
src/pyinit.jl 54.63% <30.23%> (-19.91%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e3f2ac...31ef039. Read the comment docs.

@tkf
Copy link
Member Author

tkf commented Oct 22, 2018

I'm trying to run the change 05877b9 on AppVeyor but it's not get triggered... (Tried re-open and force-push)

* `venv` support is now properly tested.  This finds a bug in
  `pythonhome_of`: `sys.base_prefix` should be used instead of
  `sys.prefix` (same for `sys.base_exec_prefix`).

* The path to Python executable passed to `pythonhome_of` was ignored
  when `python_cmd` was introduced.  This is fixed now.

* Use `python_cmd` in `find_libpython`.  This properly sets
  `PYTHONIOENCODING`.
if hasattr(sys, "base_exec_prefix"):
sys.stdout.write(sys.base_prefix)
sys.stdout.write(":")
sys.stdout.write(sys.base_exec_prefix)
Copy link
Member Author

@tkf tkf Oct 23, 2018

Choose a reason for hiding this comment

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

It looks like we should be using sys.base_prefix and sys.base_exec_prefix instead of sys.prefix and sys.exec_prefix to support venv. With this change, this PR (accidentally) solves #410 as well. The reason is commented just below: 9dde9a8#diff-d16c3d4423fa5349bf01729c8beea4cdR90

@tkf tkf closed this Oct 23, 2018
@tkf
Copy link
Member Author

tkf commented Nov 4, 2018

@stevengj I think it's good to go, unless you want to bikeshed the environment variable names PYCALL_JL_RUNTIME_PYTHON and PYCALL_JL_RUNTIME_PYTHONHOME and/or there are some problems in the code.

@stevengj
Copy link
Member

Should it be documented in the README?

@tkf
Copy link
Member Author

tkf commented Jan 24, 2019

Sure, I added a section in README.

preview: https://github.com/tkf/PyCall.jl/tree/venv#python-virtual-environment

# Fix the environment for running `python`, and setts IO encoding to UTF-8.
# If cmd is the Conda python, then additionally removes all PYTHON* and
# CONDA* environment variables.
function pythonenv(cmd::Cmd)
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it sufficient to just run python -E to ignore PYTHON*? Why do we need to remove CONDA* since we are not running conda?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I guess we don't want to ignore these variables for non-Conda 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.

pythonenv is just moved from build.jl to depsutils.jl to make it usable at runtime.

@stevengj
Copy link
Member

Why doesn't it work for Conda environments?

@tkf
Copy link
Member Author

tkf commented Jan 24, 2019

Each conda environment has its own copy of libpython (or rather a hard link). We can't change libpython path at run-time so it's not possible to support it. But maybe it's possible to do it with always_softlink option in conda. I haven't checked it.

@tkf tkf mentioned this pull request Feb 1, 2019
1 task
@stevengj stevengj merged commit 7e1c835 into JuliaPy:master Feb 6, 2019
@tkf
Copy link
Member Author

tkf commented Feb 7, 2019

Thaaaaanks!!! 🎉

@lassepe
Copy link

lassepe commented Aug 30, 2021

@tkf Do you know whether this would work with poetry? I'm running into the same issues about incompatible libpython as you pointed out for conda. I guess that is also due to the fact that poetry copies the lib?


Edit: I was able to fix it. In case someone is hitting a similar issue: The poetry environment that I setup was using a python binary build via python-build and by default those binaries don't come with a shared object. I had to explicitly build with --enable-shared (see this link)

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.

ImportError: No module named 'encodings' when using with virtualenv + python3
4 participants