clj-python / libpython-clj

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

ISSUE-253: make require-python optionally user path informed #254

Closed jjtolton closed 8 months ago

jjtolton commented 8 months ago

Closes:

https://github.com/clj-python/libpython-clj/issues/253

This PR allows for the following sort of exploratory coding style, the PYTHONPATH is managed for the user and reset after execution.

(require-python 'os 'os.path)

(def curdir (first (os.path/split *file*)))
(def data-path (str curdir  "/some/dir/data.py"))

(slurp data-path)                    ;;=> my_data="yo"
(slurp (str (os/getcwd) "/root.py")) ;;=> dadata="x"

(require-python :from *file*
                '[dir.data :path "./some"]
                :from (os/getcwd) 
                '[root :refer [dadata]])
(assert (= dir.data/my_data "yo"))
(assert (= root/dadata "x"))

Some alternatives considered:

I attempted, initially, to augment

(defn import-module
  [modname]
  (if-let [mod (PyImport_ImportModule modname)]
    (track-pyobject mod)
    (check-error-throw)))

by adding bindings for Py_SetPath() and Py_GetPath(), but I gave up on that approach. Too many segfaults.

Some difficulties encountered:

I wrote the following testing framework for the new functionality:

(defn construct-python-path [path]
  (assert (str/starts-with? path "/tmp")
          "I'm not gonna be responsible for anything outside of your /tmp directory ♡ J.")
  (let [path-parts (rest (str/split path #"/"))]
    (loop [path       (str "/" (first path-parts))
           path-parts (rest path-parts)
           created    (cond-> []
                        (not (.exists (java.io.File. "/tmp/__init__.py")))
                        (#(do (spit "/tmp/__init__.py" "")
                              (conj % "/tmp/__init__.py"))))]
      (if (seq path-parts)
        (recur (str path "/" (first path-parts))
               (rest path-parts)
               (cond-> created
                 (.mkdir (java.io.File. path))
                 (conj path)
                 (.createNewFile (java.io.File. path "__init__.py"))
                 (conj (str (java.io.File. path "__init__.py")))))

        (if-not (.exists (java.io.File. path))
          (do (.mkdir (java.io.File. path))
              (.createNewFile (java.io.File. path "__init__.py"))
              (conj created path))
          created)))))

(defn teardown-path-path [created-dirs]
  (doseq [created-dir created-dirs]
    (when (not= created-dir "/tmp")
      (assert (str/starts-with? created-dir "/tmp")
              "I'm not gonna be responsible for anything outside of your /tmp directory ♡ J.")
      (when-not (.isFile (java.io.File. created-dir))
        (.delete (java.io.File. created-dir "__init__.py")))
      (.delete (java.io.File. created-dir)))))

(defmacro make-require-test [path from-path relative-path module-symbol var val]
  `(let [module-symbol# ~module-symbol
         path#          ~path
         var#           ~var
         val#           ~val
         response#      (symbol (str module-symbol# "/" var#))
         ;; aka @(resolve #'foo/bar) aka >>> foo/bar ;;=> data
         constructed# (#'construct-python-path
                       (if (str/ends-with? path# ".py")
                         (str (.getParent (java.io.File. path#)))
                         path#))]
     (testing (str (#'require-python :from ~from-path [module-symbol# :path ~relative-path :reload true]))
       (spit path# (str var# "=" val#))
       (#'require-python :from ~from-path [module-symbol# :path ~relative-path :reload true])
       (.delete (java.io.File. path#))
       (#'teardown-path-path constructed#)
       (is (= val# @(resolve response#))))))

(require-python '[sys :bind-ns true])
(deftest require-python-from-test
  (py/set-attr! sys "dont_write_bytecode" 1)
  (make-require-test "/tmp/fo/ba.py"
                     "/tmp"
                     nil
                     'fo.ba
                     'derp
                     1)
  (make-require-test "/tmp/a/b/c/d.py"
                     "/tmp/a"
                     "a/b/c"
                     'd
                     'data
                     6)
  (make-require-test "/tmp/a/b/c/d.py"
                     "/tmp/a"
                     "b/c"
                     'd
                     'data
                     7)
  (make-require-test "/tmp/a/b/c/d.py"
                     "/tmp/a"
                     "b"
                     'c.d
                     'data
                     8)
  (make-require-test "/tmp/a/b/c/d.py"
                     "/tmp/a"
                     nil
                     'b.c.d
                     'data
                     9)
  (make-require-test "/tmp/a/b/c/d.py"
                     "/tmp/a/b"
                     nil
                     'c.d
                     'data
                     10)
  (make-require-test "/tmp/a/b/c/d.py"
                     "/tmp"
                     nil
                     'a.b.c.d
                     'dataaa
                     11)
  (make-require-test "/tmp/a/b/c/d/e/f.py"
                     "/tmp/a/b"
                     "./c/d/e"
                     'f
                     'x
                     42)
  (testing "various require-python scenarios"
    (do (.mkdir (java.io.File. "/tmp/foo"))
        (.mkdir (java.io.File. "/tmp/baz"))
        (.mkdir (java.io.File. "/tmp/baz/quux"))
        (spit "/tmp/__init__.py" "")
        (spit "/tmp/foo/__init__.py" "")
        (spit "/tmp/foo/bar.py" "a=1")
        (spit "/tmp/baz/__init__.py" "")
        (spit "/tmp/baz/quux/__init__.py" "")
        (spit "/tmp/baz/quux/derp.py" "data=42")
        (spit "/tmp/baz/yaya.py" "x=3")
        ((fn []
           (require-python  "/tmp/foo"
                            '[foo.bar :as fb :reload true :bind-ns true]
                            {:from "/tmp/baz"}
                            '[yaya :reload true :bind-ns true]
                            :from "/tmp"
                            '[baz.quux.derp :as bqd :reload true :path nil :bind-ns true])

           (is (= 42 (py/py.. bqd -data)))
           (is (= 1 (py/py.. fb -a)))
           (is (= 3 (py/py.. yaya -x))))))))

It works fine in the REPL, but it :bomb: s out with clojure -A:test as a result, I believe, of AOT compilation issues. I sunk way more time into the testing than I did into the writing. I'm not sure how to AOT dynamic file creation, so I gave up on that approach, but I'm open to suggestions.

cnuernber commented 8 months ago

Its fine with me - you tend to be on the advanced user side of things.