[-- Attachment #1: Type: text/plain, Size: 916 bytes --] Hello, I have integrated some of the ob-session-async package in ob-R.el (finally). Most of the heavy lifting has been made by Jack. So with the patch async evaluation is expected to work out of the box for :result value #+begin_src R :session *R* :results value :async :colnames yes Sys.sleep(10) as.list(1:5) #+end_src #+RESULTS: | x | |---| | 1 | | 2 | | 3 | | 4 | | 5 | But for the time being result output produces the following output. #+begin_src R :session *R* :results output :async Sys.sleep(1) print(1:5) #+end_src #+RESULTS: : > Sys.sleep(1) : > print(1:5) : [1] 1 2 3 4 5 : > 'ob_comint_async_R_end_53c0a42fed17cf78f5508e5293029430' From my understanding the async command of python does not suffer from this issue. I'm not sure if the issue needs to be solve at the ob-R.el or at the ob-core.el. I welcome any comments on this patch or any idea to move it forward. Best regards, Jeremie [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: async in ob-R --] [-- Type: text/x-diff, Size: 10655 bytes --] From 51e1efb75e15fab348fd5a2c8b5fb5c1dbbf4d1c Mon Sep 17 00:00:00 2001 From: Jeremie Juste <djj@debian-BULLSEYE-live-builder-AMD64> Date: Sun, 26 Sep 2021 18:25:23 +0200 Subject: [PATCH] lisp/ob-R: Async evaluation in R * lisp/ob-R.el `ob-session-async-R-indicator': Add constant representing a prefix R to identity session. (ob-session-async-org-babel-R-evaluate-session):New function to evaluate R src block asynchrously. (ob-session-async-R-value-callback): New function that callback the result of the asynchronous evaluation. (org-babel-R-evaluate): Add `async' parameter and call ob-session-async-org-babel-R-evaluate-session if `async' parameter is present. (org-babel-execute:R): Call (org-babel-comint-use-async) to check if async is among `params' and add async parameter to (org-babel-R-evaluate). * testing/lisp/test-ob-R.el: Add 7 more tests for async evaluations, also taken from ob-session-async package. This is almost a carbon copy of the package of ob-session-async of Jack Kamm. The original source code can be found https://github.com/jackkamm/ob-session-async Please refer to the following thread to trace back the discussion on async evaluation in R. https://list.orgmode.org/87eed9g9p6.fsf@gmail.com/ --- lisp/ob-R.el | 90 +++++++++++++++++++++++++++++++++++-- testing/lisp/test-ob-R.el | 94 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 3 deletions(-) diff --git a/lisp/ob-R.el b/lisp/ob-R.el index 2d9073cee..299ccdf1d 100644 --- a/lisp/ob-R.el +++ b/lisp/ob-R.el @@ -158,6 +158,7 @@ This function is called by `org-babel-execute-src-block'." (save-excursion (let* ((result-params (cdr (assq :result-params params))) (result-type (cdr (assq :result-type params))) + (async (org-babel-comint-use-async params)) (session (org-babel-R-initiate-session (cdr (assq :session params)) params)) (graphics-file (and (member "graphics" (assq :result-params params)) @@ -184,7 +185,8 @@ This function is called by `org-babel-execute-src-block'." (cdr (assq :colname-names params)) colnames-p)) (or (equal "yes" rownames-p) (org-babel-pick-name - (cdr (assq :rowname-names params)) rownames-p))))) + (cdr (assq :rowname-names params)) rownames-p)) + async))) (if graphics-file nil result)))) (defun org-babel-prep-session:R (session params) @@ -371,11 +373,14 @@ Has four %s escapes to be filled in: 4. The name of the file to write to") (defun org-babel-R-evaluate - (session body result-type result-params column-names-p row-names-p) + (session body result-type result-params column-names-p row-names-p async) "Evaluate R code in BODY." (if session + (if async + (ob-session-async-org-babel-R-evaluate-session + session body result-type result-params column-names-p row-names-p) (org-babel-R-evaluate-session - session body result-type result-params column-names-p row-names-p) + session body result-type result-params column-names-p row-names-p)) (org-babel-R-evaluate-external-process body result-type result-params column-names-p row-names-p))) @@ -468,6 +473,85 @@ Insert hline if column names in output have been requested." (error "Could not parse R result")) result)) + +;;; async evaluation + +(defconst ob-session-async-R-indicator "'ob_comint_async_R_%s_%s'") + +(defun ob-session-async-org-babel-R-evaluate-session + (session body result-type result-params column-names-p row-names-p) + "Asynchronously evaluate BODY in SESSION. +Returns a placeholder string for insertion, to later be replaced +by `org-babel-comint-async-filter'." + (org-babel-comint-async-register + session (current-buffer) + "^\\(?:[>.+] \\)*\\[1\\] \"ob_comint_async_R_\\(.+?\\)_\\(.+\\)\"$" + 'org-babel-chomp + 'ob-session-async-R-value-callback) + (cl-case result-type + (value + (let ((tmp-file (org-babel-temp-file "R-"))) + (with-temp-buffer + (insert + (org-babel-chomp body)) + (let ((ess-local-process-name + (process-name (get-buffer-process session)))) + (ess-eval-buffer nil))) + (with-temp-buffer + (insert + (mapconcat + 'org-babel-chomp + (list (format org-babel-R-write-object-command + (if row-names-p "TRUE" "FALSE") + (if column-names-p + (if row-names-p "NA" "TRUE") + "FALSE") + ".Last.value" + (org-babel-process-file-name tmp-file 'noquote)) + (format ob-session-async-R-indicator + "file" tmp-file)) + "\n")) + (let ((ess-local-process-name + (process-name (get-buffer-process session)))) + (ess-eval-buffer nil))) + tmp-file)) + (output + (let ((uuid (md5 (number-to-string (random 100000000)))) + (ess-local-process-name + (process-name (get-buffer-process session)))) + (with-temp-buffer + (insert (format ob-session-async-R-indicator + "start" uuid)) + (insert "\n") + (insert body) + (insert "\n") + (insert (format ob-session-async-R-indicator + "end" uuid)) + (ess-eval-buffer nil)) + uuid)))) + +(defun ob-session-async-R-value-callback (params tmp-file) + "Callback for async value results. +Assigned locally to `ob-session-async-file-callback' in R +comint buffers used for asynchronous Babel evaluation." + (let* ((graphics-file (and (member "graphics" (assq :result-params params)) + (org-babel-graphical-output-file params))) + (colnames-p (unless graphics-file (cdr (assq :colnames params))))) + (org-babel-R-process-value-result + (org-babel-result-cond (assq :result-params params) + (with-temp-buffer + (insert-file-contents tmp-file) + (org-babel-chomp (buffer-string) "\n")) + (org-babel-import-elisp-from-file tmp-file '(16))) + (or (equal "yes" colnames-p) + (org-babel-pick-name + (cdr (assq :colname-names params)) colnames-p))))) + + + +;;; ob-session-async-R.el ends here + + (provide 'ob-R) ;;; ob-R.el ends here diff --git a/testing/lisp/test-ob-R.el b/testing/lisp/test-ob-R.el index c36bac9e6..1e60ae903 100644 --- a/testing/lisp/test-ob-R.el +++ b/testing/lisp/test-ob-R.el @@ -117,6 +117,8 @@ x )))) + + ;; (ert-deftest test-ob-r/output-with-error () ;; "make sure angle brackets are well formatted" ;; (let (ess-ask-for-ess-directory ess-history-file) @@ -151,6 +153,98 @@ log10(10) #+end_src" (org-babel-execute-src-block)))))) + +(ert-deftest ob-session-async-R-simple-session-async-value () + (let (ess-ask-for-ess-directory + ess-history-file + (org-babel-temporary-directory "/tmp") + (org-confirm-babel-evaluate nil)) + (org-test-with-temp-text + "#+begin_src R :session R :async yes\n Sys.sleep(.1)\n paste(\"Yep!\")\n#+end_src\n" + (should (let ((expected "Yep!")) + (and (not (string= expected (org-babel-execute-src-block))) + (string= expected + (progn + (sleep-for 0 200) + (goto-char (org-babel-where-is-src-block-result)) + (org-babel-read-result))))))))) + +(ert-deftest ob-session-async-R-simple-session-async-output () + (let (ess-ask-for-ess-directory + ess-history-file + (org-babel-temporary-directory "/tmp") + (org-confirm-babel-evaluate nil)) + (org-test-with-temp-text + "#+begin_src R :session R :results output :async yes\n Sys.sleep(.1)\n 1:5\n#+end_src\n" + (should (let ((expected "[1] 1 2 3 4 5")) + (and (not (string= expected (org-babel-execute-src-block))) + (string= expected + (progn + (sleep-for 0 200) + (goto-char (org-babel-where-is-src-block-result)) + (org-babel-read-result))))))))) + +(ert-deftest ob-session-async-R-named-output () + (let (ess-ask-for-ess-directory + ess-history-file + (org-babel-temporary-directory "/tmp") + org-confirm-babel-evaluate + (src-block "#+begin_src R :async :session R :results output\n 1:5\n#+end_src") + (results-before "\n\n#+NAME: foobar\n#+RESULTS:\n: [1] 1") + (results-after "\n\n#+NAME: foobar\n#+RESULTS:\n: [1] 1 2 3 4 5\n")) + (org-test-with-temp-text + (concat src-block results-before) + (should (progn (org-babel-execute-src-block) + (sleep-for 0 200) + (string= (concat src-block results-after) + (buffer-string))))))) + +(ert-deftest ob-session-async-R-named-value () + (let (ess-ask-for-ess-directory + ess-history-file + org-confirm-babel-evaluate + (org-babel-temporary-directory "/tmp") + (src-block "#+begin_src R :async :session R :results value\n paste(\"Yep!\")\n#+end_src") + (results-before "\n\n#+NAME: foobar\n#+RESULTS:\n: [1] 1") + (results-after "\n\n#+NAME: foobar\n#+RESULTS:\n: Yep!\n")) + (org-test-with-temp-text + (concat src-block results-before) + (should (progn (org-babel-execute-src-block) + (sleep-for 0 200) + (string= (concat src-block results-after) + (buffer-string))))))) + +(ert-deftest ob-session-async-R-output-drawer () + (let (ess-ask-for-ess-directory + ess-history-file + org-confirm-babel-evaluate + (org-babel-temporary-directory "/tmp") + (src-block "#+begin_src R :async :session R :results output drawer\n 1:5\n#+end_src") + (result "\n\n#+RESULTS:\n:results:\n[1] 1 2 3 4 5\n:end:\n")) + (org-test-with-temp-text + src-block + (should (progn (org-babel-execute-src-block) + (sleep-for 0 200) + (string= (concat src-block result) + (buffer-string))))))) + +(ert-deftest ob-session-async-R-value-drawer () + (let (ess-ask-for-ess-directory + ess-history-file + org-confirm-babel-evaluate + (org-babel-temporary-directory "/tmp") + (src-block "#+begin_src R :async :session R :results value drawer\n 1:3\n#+end_src") + (result "\n\n#+RESULTS:\n:results:\n1\n2\n3\n:end:\n")) + (org-test-with-temp-text + src-block + (should (progn (org-babel-execute-src-block) + (sleep-for 0 200) + (string= (concat src-block result) + (buffer-string))))))) + + + + (provide 'test-ob-R) ;;; test-ob-R.el ends here -- 2.30.2
Jeremie,
a question. in
> #+begin_src R :session *R* :results value :async :colnames yes
> Sys.sleep(10)
> as.list(1:5)
> #+end_src
i was surprised by =:async= standing alone, i.e., with no following
"yes" or "no". is that an org-mode "idiom"? i.e., unadorned header
arguments default to (some form of) "yes"?
cheers, Greg
Hello Greg,
>
> i was surprised by =:async= standing alone, i.e., with no following
> "yes" or "no". is that an org-mode "idiom"? i.e., unadorned header
> arguments default to (some form of) "yes"?
Many thanks for the feedback, assigning yes or no to async will work as expected.
#+begin_src R :session *R* :results value :async yes :colnames yes
Sys.sleep(10)
as.list(1:5)
#+end_src
No async process in R
#+begin_src R :session *R* :results value :async no :colnames yes
Sys.sleep(10)
as.list(1:5)
#+end_src
:async without parameters will default to yes.
#+begin_src R :session *R* :results value :async :colnames yes
Sys.sleep(10)
as.list(1:5)
#+end_src
I am not sure if it is a desirable feature or not.
Best regards,
Jeremie
hi, Jeremie, > Many thanks for the feedback, assigning yes or no to async will work > as expected. thanks for the answer. > I am not sure if it is a desirable feature or not. if this is not already idiomatic for org mode, i'd vote to require the "yes" or "no". just my 2 cents. cheers, Greg
Hi Greg and Jeremie,
Greg Minshall <minshall@umich.edu> writes:
> if this is not already idiomatic for org mode, i'd vote to require the
> "yes" or "no". just my 2 cents.
Agreed: even if a syntax is allowed, let's use the idiomatic form in
examples.
2 cts,
--
Bastien
Hi Jeremie,
Jeremie Juste <jeremiejuste@gmail.com> writes:
> I have integrated some of the ob-session-async package in ob-R.el
> (finally). Most of the heavy lifting has been made by Jack.
When this is reliable enough, feel free to commit and push.
You can also enhance it later on.
Thanks,
--
Bastien
Jeremie,
> On Sep 26, 2021, at 10:13 AM, Jeremie Juste <jeremiejuste@gmail.com> wrote:
>
> But for the time being result output produces the following output.
>
> #+begin_src R :session *R* :results output :async
> Sys.sleep(1)
> print(1:5)
> #+end_src
>
> #+RESULTS:
> : > Sys.sleep(1)
> : > print(1:5)
> : [1] 1 2 3 4 5
> : > 'ob_comint_async_R_end_53c0a42fed17cf78f5508e5293029430'
>
>
> From my understanding the async command of python does not suffer from
> this issue. I'm not sure if the issue needs to be solve at the ob-R.el
> or at the ob-core.el. I welcome any comments on this patch or any idea
> to move it forward.
It looks like you have `(setq ess-eval-visibly t)' here. I think that is a default setting.
With (setq ess-eval-visibly nil), I get
#+RESULTS:
: >
: [1] 1 2 3 4 5
: > >
which is better, but the prompts still need cleaning along the lines of `org-babel-R-evaluate-session'.
It seems like this is an ob-R.el issue.
HTH,
Chuck
Hello, On Monday, 27 Sep 2021 at 08:48, Bastien wrote: > Hi Greg and Jeremie, > > Greg Minshall <minshall@umich.edu> writes: > >> if this is not already idiomatic for org mode, i'd vote to require the >> "yes" or "no". just my 2 cents. > > Agreed: even if a syntax is allowed, let's use the idiomatic form in > examples. Many thanks for the feedback, I'll keep that in mind and use the idomatic form in examples. For the parameter :async without any values assigned to it. I'm coordinating with ob-python.el and the orginal package https://github.com/jackkamm/ob-session-async. Jack do you see the need to change it and expect :async to have the value yes or no? Best regards, Jeremie
Hello Chuck, On Monday, 27 Sep 2021 at 18:28, Berry, Charles wrote: > > It looks like you have `(setq ess-eval-visibly t)' here. I think that is a default setting. > Many thanks for the feedback. > > which is better, but the prompts still need cleaning along the lines of `org-babel-R-evaluate-session'. > > It seems like this is an ob-R.el issue. I'll look deeper into ob-R.el Best regards, Jeremie
[-- Attachment #1: Type: text/plain, Size: 707 bytes --] Hello Chuck, On Monday, 27 Sep 2021 at 18:28, Berry, Charles wrote: > > It looks like you have `(setq ess-eval-visibly t)' here. I think that is a default setting. Thanks again for your suggestion. The following patch to be applied on top of the previous one, solves the issue. #+begin_src R :session *R* :results output :async yes Sys.sleep(5) 1:5 #+end_src #+RESULTS: : [1] 1 2 3 4 5 The function (ess-eval-buffer), when the parameter VIS is nil, misled me in the sense that it inverses the value of ess-eval-visibly. Note as well that all the tests in test-ob-R.el passed including the tests for async evaluation from [1] ob-session-async. [1]: https://github.com/jackkamm/ob-session-async/. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch-2 --] [-- Type: text/x-diff, Size: 1077 bytes --] From 795cc0ebe637aa4ff148c495cf5403ba2baec242 Mon Sep 17 00:00:00 2001 From: Jeremie Juste <djj@debian-BULLSEYE-live-builder-AMD64> Date: Mon, 27 Sep 2021 22:02:17 +0200 Subject: [PATCH] ob-R.el: Patch async evaluation when :results output * lisp/ob-R.el (ob-session-async-org-babel-R-evaluate-session): Make sure that 'ess-eval-visibly' is nil before evaluating the temporary buffer, but return ess-eval-visibly to it's original state afterwards. --- lisp/ob-R.el | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lisp/ob-R.el b/lisp/ob-R.el index 299ccdf1d..188b9ac8f 100644 --- a/lisp/ob-R.el +++ b/lisp/ob-R.el @@ -526,8 +526,11 @@ by `org-babel-comint-async-filter'." (insert body) (insert "\n") (insert (format ob-session-async-R-indicator - "end" uuid)) + "end" uuid)) + (setq tmp ess-eval-visibly) + (setq ess-eval-visibly nil) (ess-eval-buffer nil)) + (setq ess-eval-visibly tmp) uuid)))) (defun ob-session-async-R-value-callback (params tmp-file) -- 2.30.2 [-- Attachment #3: Type: text/plain, Size: 70 bytes --] Thanks again to Jack for this useful feature. Best regards, Jeremie
Jeremie,
There is something in my init that doesn't play nice with this.
IOW, emacs -q and then load the minimal stuff works OK, but my usual startup does not.
#+begin_src R :session *R* :results output :async yes
Sys.sleep(5)
1:5
#+end_src
#+RESULTS:
: > >
: [1] 1 2 3 4 5
: >
It may take me a while to figure out what it is.
:-(
HTH,
Chuck
> On Sep 27, 2021, at 1:28 PM, Jeremie Juste <jeremiejuste@gmail.com> wrote:
>
> Hello Chuck,
>
> On Monday, 27 Sep 2021 at 18:28, Berry, Charles wrote:
>>
>> It looks like you have `(setq ess-eval-visibly t)' here. I think that is a default setting.
>
> Thanks again for your suggestion. The following patch to be applied on
> top of the previous one, solves the issue.
>
>
> #+begin_src R :session *R* :results output :async yes
> Sys.sleep(5)
> 1:5
> #+end_src
>
> #+RESULTS:
> : [1] 1 2 3 4 5
>
>
> The function (ess-eval-buffer), when the parameter VIS is nil, misled me in the sense that it inverses
> the value of ess-eval-visibly.
>
> Note as well that all the tests in test-ob-R.el passed including the
> tests for async evaluation from [1] ob-session-async.
>
> [1]: https://urldefense.com/v3/__https://github.com/jackkamm/ob-session-async/__;!!LLK065n_VXAQ!wfTTIQ1fHIH2f6FmnUYZow4BoKA9_bQyOgBdm4tgLfJxbDF0vVs4sTbSQ0Zm-7YjVg$ .
>
> <0001-ob-R.el-Patch-async-evaluation-when-results-output.patch>
> Thanks again to Jack for this useful feature.
>
> Best regards,
> Jeremie
Jeremie,
> On Sep 27, 2021, at 3:56 PM, Berry, Charles <ccberry@health.ucsd.edu> wrote:
>
> There is something in my init that doesn't play nice with this.
(setq ess-inject-source nil)
seems to be the culprit.
Also note, even with ess-inject-source set to t, there is an indentation issue:
#+begin_src R :session *R* :results output :async yes
Sys.sleep(2)
1:5
10:20
#+end_src
#+RESULTS:
: [1] 1 2 3 4 5
: [1] 10 11 12 13 14 15 16 17 18 19 20
HTH,
Chuck
Hi Jeremie,
> For the parameter :async without any values assigned to it. I'm
> coordinating with ob-python.el and the orginal package
> https://github.com/jackkamm/ob-session-async.
>
> Jack do you see the need to change it and expect :async to have the
> value yes or no?
I wrote the :async header to use this behavior, following conventions
from other (external) org packages with the :async header.
In particular, below are the packages I know of with ":async" -- I
believe most of them allow using ":async" instead of ":async yes", but
it's been awhile since I've checked, so I may be misremembering:
- ob-async
- ob-ipython
- ob-jupyter
- ob-ein
I can see why requiring an explicit ":async yes" might be preferable
though, and wouldn't oppose the change if there's a consensus for it.
Jack
[-- Attachment #1: Type: text/plain, Size: 946 bytes --] Hello Chuck, On Monday, 27 Sep 2021 at 23:40, Berry, Charles wrote: > Jeremie, > >> On Sep 27, 2021, at 3:56 PM, Berry, Charles <ccberry@health.ucsd.edu> wrote: >> >> There is something in my init that doesn't play nice with this. > > (setq ess-inject-source nil) Thanks for the feedback. With the following patch, I made sure that ess-inject-source is set to default before evaluating the buffer. So even if I set (setq ess-inject-source 'function-and-buffer), I get the following output. Note that I get the same output in the IESS console buffer when I execute the command following command. #+begin_src R :session *R* :results output :async yes Sys.sleep(2) 1:5 10:20 1:2 #+end_src #+RESULTS: : [1] 1 2 3 4 5 : [1] 10 11 12 13 14 15 16 17 18 19 20 : [1] 1 2 It might be good to fix this on the ESS side. I'll see what can be done, but I'd appreciate any input you might have on this. Thanks again. Best regards, Jeremie [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch3 --] [-- Type: text/x-diff, Size: 1258 bytes --] From db2ad631247a5c52d9d6f6779948f6d0cf34c698 Mon Sep 17 00:00:00 2001 From: Jeremie Juste <djj@debian-BULLSEYE-live-builder-AMD64> Date: Tue, 28 Sep 2021 09:04:25 +0200 Subject: [PATCH] ob-R.el: Patch async evaluation when :results output * lisp/ob-R.el (ob-session-async-org-babel-R-evaluate-session): Make sure that `ess-inject-source' is set to the default 'function-and-buffer before running (ess-eval-buffer). Return `ess-inject-source' to its user-specified state afterwards. --- lisp/ob-R.el | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lisp/ob-R.el b/lisp/ob-R.el index 188b9ac8f..7e050c094 100644 --- a/lisp/ob-R.el +++ b/lisp/ob-R.el @@ -528,9 +528,13 @@ by `org-babel-comint-async-filter'." (insert (format ob-session-async-R-indicator "end" uuid)) (setq tmp ess-eval-visibly) + (setq user-inject-src-param ess-inject-source) (setq ess-eval-visibly nil) + (setq ess-inject-source 'function-and-buffer) (ess-eval-buffer nil)) - (setq ess-eval-visibly tmp) + (setq ess-eval-visibly tmp) + (setq ess-inject-source user-inject-src-param) + uuid)))) (defun ob-session-async-R-value-callback (params tmp-file) -- 2.30.2
Jeremie,
> On Sep 28, 2021, at 12:34 AM, Jeremie Juste <jeremiejuste@gmail.com> wrote:
>
> Thanks for the feedback. With the following patch, I made sure that
> ess-inject-source is set to default before evaluating the buffer.
>
> So even if I set
> (setq ess-inject-source 'function-and-buffer), I get the following
> output. Note that I get the same output in the IESS console buffer when
> I execute the command following command.
>
> #+begin_src R :session *R* :results output :async yes
> Sys.sleep(2)
> 1:5
>
> 10:20
> 1:2
> #+end_src
OK. The patch works when applied on top of the previous 2 (but the second one has the same name, so there is that to watch out for).
However, I think we are not quite home free. With `:async no', this works as expected:
#+begin_src R :session *R* :results output :async no
Sys.sleep(2)
1:5
10:
a
1:2
#+end_src
#+RESULTS:
:
: [1] 1 2 3 4 5
:
: Error: object 'a' not found
:
: [1] 1 2
But changing to `:async yes', the error aborts in a way that omits the output.
HTH,
Chuck
Hello Chuck, > OK. The patch works when applied on top of the previous 2 (but the second one has the same name, so there is that to watch out for). Thanks for the feedback, I'll make sure to provide unique names for patches in the future. > > However, I think we are not quite home free. With `:async no', this works as expected: > > #+begin_src R :session *R* :results output :async no > Sys.sleep(2) > 1:5 > 10: > a > 1:2 > #+end_src > > #+RESULTS: > : > : [1] 1 2 3 4 5 > : > : Error: object 'a' not found > : > : [1] 1 2 > > But changing to `:async yes', the error aborts in a way that omits the output. Interesting, I haven't thought about errors cases enough. Async process will be on the 9.5 release and this issue will be next on the todo list. Many thanks again for the feedback. It does help, Jeremie
Hi Jeremie, Many thanks for bringing this over the finish line! I'm very glad it made it into Org 9.5. All the tests passed on my end, and and I successfully ran a few async R blocks without any issues. I do have some suggestions for code style below. They apply to both the original patch, as well as followup fixes that have been committed since. > +(defconst ob-session-async-R-indicator "'ob_comint_async_R_%s_%s'") > + > +(defun ob-session-async-org-babel-R-evaluate-session For consistency with the rest of ob-R.el, as well as with the async functions in ob-python, I suggest using a prefix of "org-babel-R-async-". So "ob-session-async-org-babel-R-evaluate-session" would become "org-babel-R-async-evaluate-session", etc. > +(ert-deftest ob-session-async-R-simple-session-async-value () Again, for consistency I suggest renaming the test functions so they are prefixed with "test-ob-R/". For example, "ob-session-async-R-simple-session-async-value" could become "test-ob-R/async-session-simple-value". This would also allow easily running all the R tests using "BTEST_RE='.*ob-R.*' make test". > + (setq user-inject-src-param ess-inject-source) > (setq ess-eval-visibly nil) > + (setq ess-inject-source 'function-and-buffer) > (ess-eval-buffer nil)) > - (setq ess-eval-visibly tmp) > + (setq ess-eval-visibly tmp) > + (setq ess-inject-source user-inject-src-param) Rather than using setq, it would be better to let bind these like so: (let ((ess-eval-visibly nil) (ess-inject-source 'function-and-buffer)) ...code in here... ) This temporarily sets the variables within the let-block, then resets them to their original values afterwards. Then you can also remove the temporary variables "user-inject-src-param" and "ess-eval-visibly-tmp" as well as their defvars at the top of the file. Actually, since the code is already in a let-block, you can simply add these variables to the existing let-statement. > + (list (format org-babel-R-write-object-command > + (if row-names-p "TRUE" "FALSE") > + (if column-names-p > + (if row-names-p "NA" "TRUE") > + "FALSE") > + ".Last.value" > + (org-babel-process-file-name tmp-file 'noquote)) Some parts of ob-session-async-org-babel-R-evaluate-session, such as the above, are duplicated from org-babel-R-evaluate-session; it would be good to reduce duplication by abstracting these out to separate functions. Thanks again for porting this, and for taking care of ob-R.el in general. All the best, Jack
Hi Jeremie and Chuck,
>> But changing to `:async yes', the error aborts in a way that omits the output.
>
> Interesting, I haven't thought about errors cases enough. Async process
> will be on the 9.5 release and this issue will be next on the todo list.
> Many thanks again for the feedback.
I agree async eval should handle error outputs better. Errors are also
not handled well in ob-python's async eval at the moment. Certainly
something to improve in the future.
Best,
Jack
I just noticed one more thing, regarding ess-eval-visibly etc: >> + (setq user-inject-src-param ess-inject-source) >> (setq ess-eval-visibly nil) >> + (setq ess-inject-source 'function-and-buffer) >> (ess-eval-buffer nil)) >> - (setq ess-eval-visibly tmp) >> + (setq ess-eval-visibly tmp) >> + (setq ess-inject-source user-inject-src-param) > > Rather than using setq, it would be better to let bind these like so: > > (let ((ess-eval-visibly nil) > (ess-inject-source 'function-and-buffer)) > ...code in here... > ) I noticed these variables are only reset to defaults when ":results output". It may also be necessary to set them as well for the case of ":results value". In my original implementation [1], I set "ess-eval-visibly" within the wrapping advice function, so it applied to all cases. [1] https://github.com/jackkamm/ob-session-async/blob/master/lisp/ob-session-async-R.el