emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] ob-python: Fix async evaluation
@ 2023-07-11  3:40 Liu Hui
  2023-07-11  9:14 ` Ihor Radchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Liu Hui @ 2023-07-11  3:40 UTC (permalink / raw)
  To: emacs-orgmode

[-- 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-python: Fix async evaluation
  2023-07-11  3:40 [PATCH] ob-python: Fix async evaluation Liu Hui
@ 2023-07-11  9:14 ` Ihor Radchenko
  2023-07-12  4:51   ` Jack Kamm
  0 siblings, 1 reply; 7+ messages in thread
From: Ihor Radchenko @ 2023-07-11  9:14 UTC (permalink / raw)
  To: Liu Hui, Jack Kamm; +Cc: emacs-orgmode

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>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-python: Fix async evaluation
  2023-07-11  9:14 ` Ihor Radchenko
@ 2023-07-12  4:51   ` Jack Kamm
  2023-07-12 10:11     ` Liu Hui
  0 siblings, 1 reply; 7+ messages in thread
From: Jack Kamm @ 2023-07-12  4:51 UTC (permalink / raw)
  To: Ihor Radchenko, Liu Hui; +Cc: emacs-orgmode

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-python: Fix async evaluation
  2023-07-12  4:51   ` Jack Kamm
@ 2023-07-12 10:11     ` Liu Hui
  2023-07-12 21:58       ` Jack Kamm
  0 siblings, 1 reply; 7+ messages in thread
From: Liu Hui @ 2023-07-12 10:11 UTC (permalink / raw)
  To: Jack Kamm; +Cc: Ihor Radchenko, emacs-orgmode

[-- 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-python: Fix async evaluation
  2023-07-12 10:11     ` Liu Hui
@ 2023-07-12 21:58       ` Jack Kamm
  2023-07-13  9:59         ` Liu Hui
  0 siblings, 1 reply; 7+ messages in thread
From: Jack Kamm @ 2023-07-12 21:58 UTC (permalink / raw)
  To: Liu Hui; +Cc: Ihor Radchenko, emacs-orgmode

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...)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-python: Fix async evaluation
  2023-07-12 21:58       ` Jack Kamm
@ 2023-07-13  9:59         ` Liu Hui
  2023-07-14  0:29           ` Jack Kamm
  0 siblings, 1 reply; 7+ messages in thread
From: Liu Hui @ 2023-07-13  9:59 UTC (permalink / raw)
  To: Jack Kamm; +Cc: Ihor Radchenko, emacs-orgmode

[-- 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-python: Fix async evaluation
  2023-07-13  9:59         ` Liu Hui
@ 2023-07-14  0:29           ` Jack Kamm
  0 siblings, 0 replies; 7+ messages in thread
From: Jack Kamm @ 2023-07-14  0:29 UTC (permalink / raw)
  To: Liu Hui; +Cc: Ihor Radchenko, emacs-orgmode

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...


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-07-14  0:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-11  3:40 [PATCH] ob-python: Fix async evaluation Liu Hui
2023-07-11  9:14 ` Ihor Radchenko
2023-07-12  4:51   ` Jack Kamm
2023-07-12 10:11     ` Liu Hui
2023-07-12 21:58       ` Jack Kamm
2023-07-13  9:59         ` Liu Hui
2023-07-14  0:29           ` Jack Kamm

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).