[PATCH] ob-python: Fix async evaluation

[PATCH] ob-python: Fix async evaluation

[Liu Hui]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

Hi,

To reproduce the bug:

1. create test.org:
──────────────────✀──────────────────
#+begin_src python :session "*Python 3*" :async t
1
#+end_src

# Local Variables:
# python-shell-buffer-name: "Python 3"
# End:
──────────────────✀──────────────────

2. emacs -Q -L <path_to_latest_org> --eval "(require 'ob-python)"

3. Open test.org, then start a python shell with M-x run-python, which
should create a buffer named "*Python 3*"

4. Press C-c C-c on the src block. Then an error "No inferior Python
process running" is shown.

[-- Attachment #2: 0001-ob-python-Fix-async-evaluation.patch --]
[-- Type: text/x-patch, Size: 2880 bytes --]

From 75ca16a21fe409aeb37b9bf0d97895c00f38d466 Mon Sep 17 00:00:00 2001
From: Liu Hui <liuhui1610@gmail.com>
Date: Tue, 11 Jul 2023 10:49:07 +0800
Subject: [PATCH] ob-python: Fix async evaluation

* lisp/ob-python.el (org-babel-python-async-evaluate-session): Bind
`python-shell-buffer-name' inside the temp buffer.
---
 lisp/ob-python.el | 47 +++++++++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 0e0539d7a..c15d45b96 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -400,28 +400,31 @@ (defun org-babel-python-async-evaluate-session
    session (current-buffer)
    "ob_comint_async_python_\\(.+\\)_\\(.+\\)"
    'org-babel-chomp 'org-babel-python-async-value-callback)
-  (let ((python-shell-buffer-name (org-babel-python-without-earmuffs session)))
-    (pcase result-type
-      (`output
-       (let ((uuid (org-id-uuid)))
-         (with-temp-buffer
-           (insert (format org-babel-python-async-indicator "start" uuid))
-           (insert "\n")
-           (insert body)
-           (insert "\n")
-           (insert (format org-babel-python-async-indicator "end" uuid))
-           (python-shell-send-buffer))
-         uuid))
-      (`value
-       (let ((tmp-results-file (org-babel-temp-file "python-"))
-             (tmp-src-file (org-babel-temp-file "python-")))
-         (with-temp-file tmp-src-file (insert body))
-         (with-temp-buffer
-           (insert (org-babel-python-format-session-value tmp-src-file tmp-results-file result-params))
-           (insert "\n")
-           (insert (format org-babel-python-async-indicator "file" tmp-results-file))
-           (python-shell-send-buffer))
-         tmp-results-file)))))
+  (pcase result-type
+    (`output
+     (let ((uuid (org-id-uuid)))
+       (with-temp-buffer
+         (insert (format org-babel-python-async-indicator "start" uuid))
+         (insert "\n")
+         (insert body)
+         (insert "\n")
+         (insert (format org-babel-python-async-indicator "end" uuid))
+         (let ((python-shell-buffer-name
+                (org-babel-python-without-earmuffs session)))
+           (python-shell-send-buffer)))
+       uuid))
+    (`value
+     (let ((tmp-results-file (org-babel-temp-file "python-"))
+           (tmp-src-file (org-babel-temp-file "python-")))
+       (with-temp-file tmp-src-file (insert body))
+       (with-temp-buffer
+         (insert (org-babel-python-format-session-value tmp-src-file tmp-results-file result-params))
+         (insert "\n")
+         (insert (format org-babel-python-async-indicator "file" tmp-results-file))
+         (let ((python-shell-buffer-name
+                (org-babel-python-without-earmuffs session)))
+           (python-shell-send-buffer)))
+       tmp-results-file))))
 
 (provide 'ob-python)
 
-- 
2.25.1

Re: [PATCH] ob-python: Fix async evaluation

[Ihor Radchenko]

Liu Hui <liuhui1610@gmail.com> writes:

> To reproduce the bug:
>
> 1. create test.org:
> ──────────────────✀──────────────────
> #+begin_src python :session "*Python 3*" :async t
> 1
> #+end_src
>
> # Local Variables:
> # python-shell-buffer-name: "Python 3"
> # End:
> ──────────────────✀──────────────────
>
> 2. emacs -Q -L <path_to_latest_org> --eval "(require 'ob-python)"
>
> 3. Open test.org, then start a python shell with M-x run-python, which
> should create a buffer named "*Python 3*"
>
> 4. Press C-c C-c on the src block. Then an error "No inferior Python
> process running" is shown.

Confirmed.
CCing Jack Kamm, the maintainer.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] ob-python: Fix async evaluation

[Jack Kamm]

Ihor Radchenko <yantar92@posteo.net> writes:

> Liu Hui <liuhui1610@gmail.com> writes:
>
>> To reproduce the bug:
>>
>> 1. create test.org:
>> ──────────────────✀──────────────────
>> #+begin_src python :session "*Python 3*" :async t
>> 1
>> #+end_src
>>
>> # Local Variables:
>> # python-shell-buffer-name: "Python 3"
>> # End:
>> ──────────────────✀──────────────────
>>
>> 2. emacs -Q -L <path_to_latest_org> --eval "(require 'ob-python)"
>>
>> 3. Open test.org, then start a python shell with M-x run-python, which
>> should create a buffer named "*Python 3*"
>>
>> 4. Press C-c C-c on the src block. Then an error "No inferior Python
>> process running" is shown.
>
> Confirmed.
> CCing Jack Kamm, the maintainer.

And I can confirm now as well. Thanks for reporting, and for the patch.

The patch looks good, but it would be nice to include a unit test as
well -- could you update the patch to include one, Liu Hui?

Thanks,
Jack

Re: [PATCH] ob-python: Fix async evaluation

[Liu Hui]

[-- Attachment #1: Type: text/plain, Size: 251 bytes --]

Jack Kamm <jackkamm@gmail.com> 于2023年7月12日周三 12:51写道：

> The patch looks good, but it would be nice to include a unit test as
> well -- could you update the patch to include one, Liu Hui?

OK, I have added a test to the patch.

[-- Attachment #2: 0001-ob-python-Fix-async-evaluation.patch --]
[-- Type: text/x-patch, Size: 4018 bytes --]

From 56fd5e05bc33337dc82fb825416100e75663a520 Mon Sep 17 00:00:00 2001
From: Liu Hui <liuhui1610@gmail.com>
Date: Wed, 12 Jul 2023 18:07:06 +0800
Subject: [PATCH] ob-python: Fix async evaluation

* lisp/ob-python.el (org-babel-python-async-evaluate-session): Bind
`python-shell-buffer-name' inside the temp buffer.
* testing/lisp/test-ob-python.el (test-ob-python/async-local-python-shell):
Add test.
---
 lisp/ob-python.el              | 47 ++++++++++++++++++----------------
 testing/lisp/test-ob-python.el | 15 +++++++++++
 2 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 0e0539d7a..c15d45b96 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -400,28 +400,31 @@ (defun org-babel-python-async-evaluate-session
    session (current-buffer)
    "ob_comint_async_python_\\(.+\\)_\\(.+\\)"
    'org-babel-chomp 'org-babel-python-async-value-callback)
-  (let ((python-shell-buffer-name (org-babel-python-without-earmuffs session)))
-    (pcase result-type
-      (`output
-       (let ((uuid (org-id-uuid)))
-         (with-temp-buffer
-           (insert (format org-babel-python-async-indicator "start" uuid))
-           (insert "\n")
-           (insert body)
-           (insert "\n")
-           (insert (format org-babel-python-async-indicator "end" uuid))
-           (python-shell-send-buffer))
-         uuid))
-      (`value
-       (let ((tmp-results-file (org-babel-temp-file "python-"))
-             (tmp-src-file (org-babel-temp-file "python-")))
-         (with-temp-file tmp-src-file (insert body))
-         (with-temp-buffer
-           (insert (org-babel-python-format-session-value tmp-src-file tmp-results-file result-params))
-           (insert "\n")
-           (insert (format org-babel-python-async-indicator "file" tmp-results-file))
-           (python-shell-send-buffer))
-         tmp-results-file)))))
+  (pcase result-type
+    (`output
+     (let ((uuid (org-id-uuid)))
+       (with-temp-buffer
+         (insert (format org-babel-python-async-indicator "start" uuid))
+         (insert "\n")
+         (insert body)
+         (insert "\n")
+         (insert (format org-babel-python-async-indicator "end" uuid))
+         (let ((python-shell-buffer-name
+                (org-babel-python-without-earmuffs session)))
+           (python-shell-send-buffer)))
+       uuid))
+    (`value
+     (let ((tmp-results-file (org-babel-temp-file "python-"))
+           (tmp-src-file (org-babel-temp-file "python-")))
+       (with-temp-file tmp-src-file (insert body))
+       (with-temp-buffer
+         (insert (org-babel-python-format-session-value tmp-src-file tmp-results-file result-params))
+         (insert "\n")
+         (insert (format org-babel-python-async-indicator "file" tmp-results-file))
+         (let ((python-shell-buffer-name
+                (org-babel-python-without-earmuffs session)))
+           (python-shell-send-buffer)))
+       tmp-results-file))))
 
 (provide 'ob-python)
 
diff --git a/testing/lisp/test-ob-python.el b/testing/lisp/test-ob-python.el
index 7aac87116..14dae0ef5 100644
--- a/testing/lisp/test-ob-python.el
+++ b/testing/lisp/test-ob-python.el
@@ -296,6 +296,21 @@ (ert-deftest test-ob-python/async-output-drawer ()
                      (string= (concat src-block result)
                               (buffer-string)))))))
 
+(ert-deftest test-ob-python/async-local-python-shell ()
+  ;; Disable the test on older Emacs as built-in python.el sometimes
+  ;; fail to initialize session.
+  (skip-unless (version<= "28" emacs-version))
+  (when-let ((buf (get-buffer "*Python*")))
+    (let (kill-buffer-query-functions)
+      (kill-buffer buf)))
+  (org-test-with-temp-text-in-file
+      "# -*- python-shell-buffer-name: \"Python 3\" -*-
+<point>#+begin_src python :session \"*Python 3*\" :async yes
+1
+#+end_src"
+    (run-python nil nil 'hide)
+    (should (org-babel-execute-src-block))))
+
 (provide 'test-ob-python)
 
 ;;; test-ob-python.el ends here
-- 
2.25.1

Re: [PATCH] ob-python: Fix async evaluation

[Jack Kamm]

Liu Hui <liuhui1610@gmail.com> writes:

> OK, I have added a test to the patch.

While your test works on its own, it seems to break subsequent tests
(the next test hangs).

My guess is that it has something to do with the fact that most of the
Python session tests share the same session, and ob-python is getting
confused about which session to use on the next test.

One possible fix might be to kill the Python session started by your new
test, after it's finished running.

(It is probably not the best design that the Python session tests all
share the same session...)

Re: [PATCH] ob-python: Fix async evaluation

[Liu Hui]

[-- Attachment #1: Type: text/plain, Size: 325 bytes --]

Jack Kamm <jackkamm@gmail.com> 于2023年7月13日周四 05:58写道：

> While your test works on its own, it seems to break subsequent tests
> (the next test hangs).

Thanks for pointing out the problem! I find the problem disappears
after removing the `run-python` line, and I have updated the patch
accordingly.

[-- Attachment #2: 0001-ob-python-Fix-async-evaluation.patch --]
[-- Type: text/x-patch, Size: 3985 bytes --]

From 4c552eb4eec5d84727775645cdf41acebd7fd1da Mon Sep 17 00:00:00 2001
From: Liu Hui <liuhui1610@gmail.com>
Date: Wed, 12 Jul 2023 18:07:06 +0800
Subject: [PATCH] ob-python: Fix async evaluation

* lisp/ob-python.el (org-babel-python-async-evaluate-session): Bind
`python-shell-buffer-name' inside the temp buffer.
* testing/lisp/test-ob-python.el (test-ob-python/async-local-python-shell):
Add test.
---
 lisp/ob-python.el              | 47 ++++++++++++++++++----------------
 testing/lisp/test-ob-python.el | 14 ++++++++++
 2 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 0e0539d7a..c15d45b96 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -400,28 +400,31 @@ (defun org-babel-python-async-evaluate-session
    session (current-buffer)
    "ob_comint_async_python_\\(.+\\)_\\(.+\\)"
    'org-babel-chomp 'org-babel-python-async-value-callback)
-  (let ((python-shell-buffer-name (org-babel-python-without-earmuffs session)))
-    (pcase result-type
-      (`output
-       (let ((uuid (org-id-uuid)))
-         (with-temp-buffer
-           (insert (format org-babel-python-async-indicator "start" uuid))
-           (insert "\n")
-           (insert body)
-           (insert "\n")
-           (insert (format org-babel-python-async-indicator "end" uuid))
-           (python-shell-send-buffer))
-         uuid))
-      (`value
-       (let ((tmp-results-file (org-babel-temp-file "python-"))
-             (tmp-src-file (org-babel-temp-file "python-")))
-         (with-temp-file tmp-src-file (insert body))
-         (with-temp-buffer
-           (insert (org-babel-python-format-session-value tmp-src-file tmp-results-file result-params))
-           (insert "\n")
-           (insert (format org-babel-python-async-indicator "file" tmp-results-file))
-           (python-shell-send-buffer))
-         tmp-results-file)))))
+  (pcase result-type
+    (`output
+     (let ((uuid (org-id-uuid)))
+       (with-temp-buffer
+         (insert (format org-babel-python-async-indicator "start" uuid))
+         (insert "\n")
+         (insert body)
+         (insert "\n")
+         (insert (format org-babel-python-async-indicator "end" uuid))
+         (let ((python-shell-buffer-name
+                (org-babel-python-without-earmuffs session)))
+           (python-shell-send-buffer)))
+       uuid))
+    (`value
+     (let ((tmp-results-file (org-babel-temp-file "python-"))
+           (tmp-src-file (org-babel-temp-file "python-")))
+       (with-temp-file tmp-src-file (insert body))
+       (with-temp-buffer
+         (insert (org-babel-python-format-session-value tmp-src-file tmp-results-file result-params))
+         (insert "\n")
+         (insert (format org-babel-python-async-indicator "file" tmp-results-file))
+         (let ((python-shell-buffer-name
+                (org-babel-python-without-earmuffs session)))
+           (python-shell-send-buffer)))
+       tmp-results-file))))
 
 (provide 'ob-python)
 
diff --git a/testing/lisp/test-ob-python.el b/testing/lisp/test-ob-python.el
index 7aac87116..82fbca36e 100644
--- a/testing/lisp/test-ob-python.el
+++ b/testing/lisp/test-ob-python.el
@@ -296,6 +296,20 @@ (ert-deftest test-ob-python/async-output-drawer ()
                      (string= (concat src-block result)
                               (buffer-string)))))))
 
+(ert-deftest test-ob-python/async-local-python-shell ()
+  ;; Disable the test on older Emacs as built-in python.el sometimes
+  ;; fail to initialize session.
+  (skip-unless (version<= "28" emacs-version))
+  (when-let ((buf (get-buffer "*Python*")))
+    (let (kill-buffer-query-functions)
+      (kill-buffer buf)))
+  (org-test-with-temp-text-in-file
+      "# -*- python-shell-buffer-name: \"Python 3\" -*-
+<point>#+begin_src python :session \"*Python 3*\" :async yes
+1
+#+end_src"
+    (should (org-babel-execute-src-block))))
+
 (provide 'test-ob-python)
 
 ;;; test-ob-python.el ends here
-- 
2.25.1

Re: [PATCH] ob-python: Fix async evaluation

[Jack Kamm]

Liu Hui <liuhui1610@gmail.com> writes:

> Thanks for pointing out the problem! I find the problem disappears
> after removing the `run-python` line, and I have updated the patch
> accordingly.

Thank you. I applied your patch to main branch.

I am curious why the previous version of your test was causing hanging
-- whether it indicates a bug in ob-python, or just an issue with the
testing. Would be good to look into it more...