clj-python / libpython-clj

Python bindings for Clojure
Eclipse Public License 2.0
1.05k stars 68 forks source link

More correct to set `:python-home` from `sys.base_prefix`? #189

Closed harishcm closed 2 years ago

harishcm commented 2 years ago

Hi Chris,

Many thanks for your excellent library which is changing the Clojure landscape.

I recently had an issue with autodetection of :python-home in a virtual environment (built with either Poetry or venv) and solved it by (1) activating the virtual environment, (2) setting $PYTHONHOME, and (3) calling lein $TASK while inside the virtual environment.

Out of curiosity, I dug in further, and it seems to me that it may be more correct (at least to cover the use case of virtual environments) to autodetect :python-home from sys.base_prefix instead of the current sys.prefix as used in the library function find-python-home.

My reasoning:

PEP 405 -- Python Virtual Environments specifies that for virtual environments, sys.prefix is changed to point relative to site-packages, while sys.base_prefix is set relative to the Python executable and the standard libraries.

image

In addition, this is also is also emphasized in the documentation for the sys module which states that when virtual environments are used, it is sys.base_prefix that remains pointing to the base Python installation (the one which the virtual environment was created from).

image

image

I tried the patched version of find-python-home below on my local machine with virtual environments (created by either Poetry or venv) and the autodetection of :python-home works properly.

(defn find-python-home
  [system-info & [{:keys [python-home]}]]
  (cond
    python-home
    python-home
    (seq (System/getenv "PYTHONHOME"))
    (System/getenv "PYTHONHOME")
    :else
    (:base-prefix system-info))) ; change :prefix to :base-prefix

For your kind consideration. Many thanks!

cnuernber commented 2 years ago

This is great! Solid argument- We should do it and see.

harishcm commented 2 years ago

Great to hear! If I could be of any help in the process/testing, please let me know what I could do :)

jjtolton commented 2 years ago

Why not make a PR? Would be more than welcome!

On Wed, Dec 22, 2021 at 1:39 AM Mohanadas Harish Chandar < @.***> wrote:

Great to hear! If I could be of any help in the process/testing, please let me know what I could do :)

— Reply to this email directly, view it on GitHub https://github.com/clj-python/libpython-clj/issues/189#issuecomment-999324948, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPJX47K44VMBRVI5R7WOCDUSFXCRANCNFSM5KPQLTNQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

harishcm commented 2 years ago

Just sent a PR #190 . My first one - hope it is alright. Many thanks!

cnuernber commented 2 years ago

Closed by #190