[WIP] Use preferences.jl to set python executable #945

PhilipVinc · 2021-12-05T12:51:49Z

This PR is a re-exhumation of #835, and most of the credit goes to @tkf .

An internal package PyPreferences.jl exposes a simple API (use_system(), use_conda()) to set the current python, and detects version/pythonhome so that this code can be removed from the build phase of PyCall.

Since it stores those options using Preferences.jl, changing the python executable will trigger a recompilation.
Unfortuantely, as Preferences.jl only works on Julia>=1.6, this means that either we leave in place the old build machinery for older Julia versions and switch to the new one only on recent Julia versions, or we drop support for Julia <1.6. This would involve considerable more work and would make testing more complex.

Is dropping support an option?

I'm going to put this up so that I can see how badly CI fails..

Another thing to consider: With respect to PkgCompiler.jl, do we want PyCall to support changing the executable path/version after compilation or that is not required?

PhilipVinc · 2021-12-05T13:49:27Z

JuliaLang/Pkg.jl#2500

codecov-commenter · 2021-12-05T14:13:58Z

Codecov Report

Merging #945 (7c86096) into master (e3e3008) will decrease coverage by 0.52%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #945      +/-   ##
==========================================
- Coverage   68.33%   67.80%   -0.53%     
==========================================
  Files          20       21       +1     
  Lines        2018     2047      +29     
==========================================
+ Hits         1379     1388       +9     
- Misses        639      659      +20

Flag	Coverage Δ
unittests	`67.80% <80.00%> (-0.53%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/PyCall.jl	`69.87% <ø> (-0.83%)`	⬇️
src/startup_helpers.jl	`75.00% <ø> (ø)`
src/startup.jl	`46.15% <66.66%> (-6.98%)`	⬇️
src/pyinit.jl	`84.09% <100.00%> (+0.41%)`	⬆️
src/pyeval.jl	`70.27% <0.00%> (-1.81%)`	⬇️
src/pybuffer.jl	`60.67% <0.00%> (-0.44%)`	⬇️
src/conversions.jl	`63.08% <0.00%> (-0.25%)`	⬇️
... and 2 more

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 e3e3008...7c86096. Read the comment docs.

PhilipVinc · 2021-12-05T14:33:53Z

TODO:

cleanup where things like find_libpython should live. PyPreferences or PyCall?
PkgCompiler tests are broken but I need to figure out why.

tkf · 2021-12-06T23:41:44Z

cleanup where things like find_libpython should live.

I'd suggest PyPreferences. I think it'd be nice if all the code currently in the deps is in some functions of PyPreferences. It makes testing and debugging easier. Maybe we can even put some troubleshooting UI in it in the future based on the libpython discovery code.

@stevengj What do you think? You seemed to prefer merging PyPreferences to PyCall instead, in #835.

PhilipVinc · 2021-12-06T23:49:49Z

I'd also like to know if dripping pre 1.6 compatibility is on the table.

stevengj · 2021-12-08T14:30:11Z

I think it's fine to drop 1.6 support going forward.

gitignore

remove artifact remove build phase start adapting CI fix ci fix ci fix ci fix ci fix ci conda fixes fix conda ci stop testing on 1.0 remove testers fix fix aot ci fixes dont test build fixes fix venv test refactor PyPreferences use env variable pyprefs project fix fixes stuff test fix conda revert changes fix cng fix python version custom which to work in julia 1.6 make test work with python 2 fixup docs

PhilipVinc · 2022-01-19T08:48:39Z

One problem I encountered is the following: Right now the preference for python can be set both to a command in the PATH or to a full path to an executable.

However, if an user changes the PATH, in order to get a different python version/venv on different folders (this happens to me using direnv) but the executable name stays the same, this will not trigger recompilation, however we would need to recompile in this case.

One thing that can be done is to normalise the python executable to a path before storing it in the preferences.
However this does not completely avoid the problem: the user could still manually set the python executable python3 by hand in LocalPreferences.toml.
Alternatively, we could add a _fullpath key to preferences, and warn users not to edit it, but that seems a bad approach.

A better way would be to add the hash of the current python executable path to the cache invalidation mechanism, so that if the executable changed we trigger a recompile, but I do not know if that is possible (maybe @StefanKarpinski who worked on this knows?)..

--

Lastly: I don't have access to a windows machine to test the failures of venv on windows. If somebody where to step in/help, it would be helpful.
Also, I don't understand why the AOT tests are failing.

StefanKarpinski · 2022-01-20T16:23:00Z

I think @staticfloat would be the person to comment since he implemented Preferences (I consulted on design but didn't do any of the implementation).

staticfloat · 2022-01-20T18:02:50Z

Here's how I would address this:

When you save the preference out, you always normalize it to a full path. If the user edits a TOML to change the value to a PATH-relative basename, that will trigger recompilation, and while compiling (e.g. at the top level) you will normalize and save it out again. E.g. something like:

function save_python_path(path::String)
    if !isabspath(path)
        which_path = Sys.which(path)
        if which_path === nothing
            error("Couldn't find python executable at $(path)")
        end
    end
    @set_preferences!("PYTHON" => path)
end

python_path = @get_preference("PYTHON")
if !isabspath(python_path)
    save_python_path(python_path)
end

Base.include_dependency(python_path)

So usage might go something like this:

User installs package, compilation happens with default python path (whatever that is).
User sets the python path to python, which gets normalized and saved. Next time it is loaded it recompiles, but after that it's fine.
User modifies the saved preferences manually, which causes recompilation. During recompilation, the path gets normalized and saved again. Next time it is loaded, it does not recompile.

If you want to recompile if the python executable itself changes, the best we can do is to use Base.include_dependency(python_path), which will trigger recompilation if the mtime of that file changes from the last time we compiled.

xukai92 · 2022-02-28T11:54:44Z

I was trying to test this PR but with no success---do I need a different version of pyjulia or do I mis-understand what this PR solves (I thought it's to fix the statically-linked Python issue)?

What I tried was dev this branch locally so that my julia picks up this PR and attempt to load a statically-linked Python.
This gave me same error (the one suggests passing compiled_modules=False) as before when I using the master branch of PyCall.jl.

MilesCranmer · 2023-07-02T23:02:58Z

Ping. What's the status of this PR?

This is badly needed. Downstream, several users have ran into bugs in my packages because of this:

Various installation issues MilesCranmer/PySR#262
[BUG] Installation issue: const PyCall = Base.require MilesCranmer/PySR#287
[BUG] Errors when building PyCall.jl MilesCranmer/PySR#257
[BUG] Can't re-build PyCall.jl after upgrading Python MilesCranmer/PySR#222
[BUG] julia.tools.PyCallInstallError: Precompiling PyCall failed. MilesCranmer/PySR#140
Attempts at downstream patches:
- Include Python and Julia version in PySR env MilesCranmer/PySR#365
- Self-repairing incremental installs MilesCranmer/PySR#363

PhilipVinc · 2023-07-03T06:47:26Z

Im not working in Julia anymore unfortunately, as Jax has captured my attention and that of my colleagues.

Feel free to pick it up. I remember there being agreement in the fact that this should be done, but it was hard to get hold of the maintainers...

MilesCranmer · 2023-07-04T13:12:51Z

Sad to hear you left Julia (why not both 😉?) but thanks for updating me on the PR status.

For others needing something like this due to PyCall.jl build issues (e.g., if you change Python version) – you might be interested in a similar workaround to the one implemented in PySR: MilesCranmer/PySR#363. Thanks @mkitti for the idea.

Basically:

import Pkg
Pkg.activate("pysr..."; shared=true)
ENV["PYTHON"] = "/path/to/python"
Pkg.build("PyCall")

will trigger PyCall to rebuild when you switch Python versions.

It doesn't fix things if you switch back and forth Python versions, but it's good enough for my target demographic who might upgrade Python once a year and wonder why PySR is breaking.

tkf mentioned this pull request Jan 14, 2022

Use julia_project to manage Julia dependency SciML/diffeqpy#100

Closed

PhilipVinc force-pushed the pv/preferences branch from 667ed10 to 6a92054 Compare January 18, 2022 22:32

PhilipVinc added 4 commits January 19, 2022 00:03

update gitignore

5467c72

gitignore

use pyPreferences

9dfa3c2

add tmate

d9da069

PhilipVinc force-pushed the pv/preferences branch from 3218757 to d9da069 Compare January 18, 2022 23:04

fix

7c86096

tkf mentioned this pull request Jan 21, 2022

Turn on compiled_modules=False automatically? JuliaPy/pyjulia#487

Open

tkf mentioned this pull request Jan 30, 2022

PyCall cannot find ctypes on python 3.10.2 #963

Closed

tkf mentioned this pull request Mar 13, 2022

PyJulia/PyCall.jl not working well with new compilation cache management in Julia 0.7/Pkg3.jl JuliaLang/julia#28518

Closed

MilesCranmer mentioned this pull request Feb 22, 2023

Package compiler MilesCranmer/PySR#230

Closed

schlichtanders mentioned this pull request Mar 1, 2023

How to access Python variables from %%Julia in ipython? JuliaPy/PythonCall.jl#278

Closed

MilesCranmer mentioned this pull request Jul 2, 2023

[WIP] Optionally store deps.jl in a different directory #991

Open

MilesCranmer mentioned this pull request Jul 2, 2023

Include Python and Julia version in PySR env MilesCranmer/PySR#365

Closed

xukai92 mentioned this pull request Jul 4, 2023

Self-repairing incremental installs MilesCranmer/PySR#363

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Use preferences.jl to set python executable #945

[WIP] Use preferences.jl to set python executable #945

PhilipVinc commented Dec 5, 2021 •

edited

Loading

PhilipVinc commented Dec 5, 2021

codecov-commenter commented Dec 5, 2021 •

edited

Loading

PhilipVinc commented Dec 5, 2021 •

edited

Loading

tkf commented Dec 6, 2021

PhilipVinc commented Dec 6, 2021

stevengj commented Dec 8, 2021

PhilipVinc commented Jan 19, 2022 •

edited

Loading

StefanKarpinski commented Jan 20, 2022

staticfloat commented Jan 20, 2022

xukai92 commented Feb 28, 2022

MilesCranmer commented Jul 2, 2023 •

edited

Loading

PhilipVinc commented Jul 3, 2023

MilesCranmer commented Jul 4, 2023 •

edited

Loading

[WIP] Use preferences.jl to set python executable #945

Are you sure you want to change the base?

[WIP] Use preferences.jl to set python executable #945

Conversation

PhilipVinc commented Dec 5, 2021 • edited Loading

PhilipVinc commented Dec 5, 2021

codecov-commenter commented Dec 5, 2021 • edited Loading

Codecov Report

PhilipVinc commented Dec 5, 2021 • edited Loading

tkf commented Dec 6, 2021

PhilipVinc commented Dec 6, 2021

stevengj commented Dec 8, 2021

PhilipVinc commented Jan 19, 2022 • edited Loading

StefanKarpinski commented Jan 20, 2022

staticfloat commented Jan 20, 2022

xukai92 commented Feb 28, 2022

MilesCranmer commented Jul 2, 2023 • edited Loading

PhilipVinc commented Jul 3, 2023

MilesCranmer commented Jul 4, 2023 • edited Loading

PhilipVinc commented Dec 5, 2021 •

edited

Loading

codecov-commenter commented Dec 5, 2021 •

edited

Loading

PhilipVinc commented Dec 5, 2021 •

edited

Loading

PhilipVinc commented Jan 19, 2022 •

edited

Loading

MilesCranmer commented Jul 2, 2023 •

edited

Loading

MilesCranmer commented Jul 4, 2023 •

edited

Loading