From: "Charles C. Berry" <ccberry@ucsd.edu>
To: Aaron Ecay <aaronecay@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: R code block produces only partial output
Date: Sat, 23 Aug 2014 17:10:16 -0700 [thread overview]
Message-ID: <alpine.OSX.2.00.1408231558290.2651@charles-berrys-macbook.local> (raw)
In-Reply-To: <87oavkp2xa.fsf@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 5044 bytes --]
Aaron,
Some comments inline.
A patch that modifies what you have done is attached. Apart from adding
`org-babel-local-file-name' in two places, I think it is functionally
that same as yours, but I find the R code easier to follow.
On Sat, 16 Aug 2014, Aaron Ecay wrote:
> Hi Chuck,
>
> Thanks for your feedback.
>
> 2014ko abuztuak 16an, "Charles C. Berry"-ek idatzi zuen:
>> Aaron,
>>
>> I think doing something along these lines makes sense. evaluate() obviates
>> the need for tryCatch() and capture.output(), and it makes customizing
>> error/warning/message stuff clean. Its use might also lead to cleaner code
>> for R graphics handling.
>>
>> But adding a dependency on the evaluate package is a significant step.
>> This could be a nuisance for users whose code runs just fine right now. It
>> would need to be installed anywhere R is executed, e.g. in remote
>> sessions. The user would need to install it in her private directory if it
>> is not on the system; some shops actually discourage this.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> That’s ... special. Do you have experience with such environments?
Not really. Just seen complaints...
>
>>
>> So, making it optional might be necessary.
>
> I don’t like this idea; it means that we’d have to support the old,
> hacky way of doing things indefinitely. I’d like to hear more from
> people for whom local installation of R packages creates an issue.
>
The old hacky way works pretty well most of the time. I suspect that it
will be hard to get folks to really test a new ob-R.el in advance of its
moving to maint. ./lisp/test-ob-R.el only has one :session src block and
a few other src blocks...
I was thinking of having a drop in replacement for the each of the two
functions that are changed and maybe selecting which version to use with
defalias. If there is a slicker way to enable a user to revert to an
earlier version without having good git skills, fine. But I think the
possibility of breaking something that someone needs and not noticing till
the changes go to maint is high.
>>
>> ---
>>
>> I looked at the patch briefly. Some comments:
>>
>> - You can ditch tryCatch and capture.output. You might browse
>> knitr:::block_exec to see how it uses evaluate().
>
> I’m not sure I see what you mean. The tryCatch exists to ensure that
> the sentinel file is created, to signal emacs that the R code is done
> running. It might not be strictly necessary, as long as the rest of
> babel’s injected code is error-free (evaluate takes care of catching
> errors in user code). But emacs will busy-wait indefinitely if the
> creation of that file does not happen, so I have tried to play it safe.
>
Not sure I get why the sentinel file is needed, but the patch here uses
on.exit(file.create(...)) to ensure that that file is created. One hiccup
(not sure if it exists in master,too) is that starting a remote session
and then trying to run src blocks from a buffer for a local file will hang
(because a local temp file is used for sentinel). So there is still stuff
to do.
> As for capture.output, the knitr function you reference is doing a lot
> of heavy lifting; I don’t understand it all. I just need to get the
> result of evaluate() into a file somehow. There are other ways of doing
> this than capture.output + replay, but it seems like they’d be just as
> complicated.
After looking more at evaluate and that knitr function, I am not keen to
replace replay() to obviate sink() or capture.output(). I had thought that
the output_handler arg offered more potential that it actually does. If
you ever feel the need to revise how graphics handling is done, it might
be worth working thru what Yihui does there.
>
>>
>> - Wrap the code in local() to keep objects you create from persisting
>> where they might not be wanted. I think using
>> local({ res <- evaluate(input, envir=parent.frame(2),...); <...>})
>> will get assignments from `input' properly placed.
>
> I omitted to rm() .org.eval.result for debugging purposes, but in the
> final patch I will do so. I much prefer an explicit variable creation /
> rm() to environment-hacking.
>
Putting objects in userspace is considered poor practice. FWIW, CRAN
disallows it. `environment-hacking' as you call it underlies much of R. If
you must create and hold variables use the attach(NULL,name="org-vars")
trick.
>>
>> - let stop_on_error, keep_warning, and keep_message args be customizable
>> or depend on a header arg. (Then I can stop wrapping require() in
>> suppressPackageStartupMessages() which I always misspell. Argh!)
>
> I don’t like this, since it would not carry over to the other 3/4
> cases (value results in a session; either type of result outside of a
> session). I’d like to get this patch working first, and then focus on
> the others. It may be that we want to move to using the evaluate
> package in all cases, in which case this suggestion would be workable.
Fair enough.
Best,
Chuck
[-- Attachment #2: modify AE's session-results-output patch --]
[-- Type: TEXT/PLAIN, Size: 2076 bytes --]
From e51c092cc455f9deee8d3d5b413493aff84b38c3 Mon Sep 17 00:00:00 2001
From: chasberry <ccberry@ucsd.edu>
Date: Sat, 23 Aug 2014 15:54:55 -0700
Subject: [PATCH 2/2] merge CCBs version
---
lisp/ob-R.el | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 08ef9c2..f828bf2 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -383,6 +383,17 @@ last statement in BODY, as elisp."
(defvar ess-eval-visibly-p)
+(defvar org-babel-R-session-cmd
+ "local({
+ on.exit(file.create(%S))
+ sink(%S)
+ replay(
+ Filter(Negate(is.source),
+ evaluate(%S, envir=parent.frame(2), new_device = FALSE,
+ stop_on_error = 0L)))
+ sink()})"
+"format string for an `R :session :results output' regime.")
+
(defun org-babel-R-evaluate-session
(session body result-type result-params column-names-p row-names-p)
"Evaluate BODY in SESSION.
@@ -419,24 +430,10 @@ last statement in BODY, as elisp."
(org-babel-comint-eval-invisibly-and-wait-for-file
session sentinel-file
(format
-"
-tryCatch(.org.eval.result <- evaluate(%S, new_device = FALSE, stop_on_error = 0L),
- finally = {
- capture.output(
- withCallingHandlers(
- replay(Filter(function (x) !inherits(x, \"source\"),
- .org.eval.result)),
- message = function (x) {
- cat(x$message);
- invokeRestart(\"muffleMessage\")
- }),
- file=%S)
- file.create(%S)
- })
-"
- (org-babel-chomp body)
- output-file
- sentinel-file))
+ org-babel-R-session-cmd
+ (org-babel-local-file-name sentinel-file)
+ (org-babel-local-file-name output-file)
+ (org-babel-chomp body)))
(with-temp-buffer
(insert-file-contents output-file)
(goto-char (point-min))
--
1.8.5.2 (Apple Git-48)
next prev parent reply other threads:[~2014-08-24 0:10 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-04 11:18 R code block produces only partial output Andreas Kiermeier
2014-08-04 11:53 ` Eric Schulte
2014-08-04 12:23 ` Andreas Kiermeier
2014-08-04 13:10 ` Eric Schulte
2014-08-05 0:46 ` Andreas Kiermeier
2014-08-05 4:00 ` John Hendy
2014-08-05 4:31 ` Andreas Kiermeier
2014-08-05 18:05 ` Charles Berry
2014-08-05 19:02 ` Eric Schulte
2014-08-05 19:11 ` John Hendy
2014-08-05 19:57 ` Nick Dokos
2014-08-05 20:10 ` Nick Dokos
2014-08-05 22:21 ` Charles C. Berry
2014-08-06 3:32 ` Aaron Ecay
2014-08-06 11:30 ` Eric Schulte
2014-08-07 6:00 ` Aaron Ecay
2014-08-07 17:42 ` Charles C. Berry
2014-08-07 18:06 ` Aaron Ecay
2014-08-07 18:42 ` Charles C. Berry
2014-08-07 19:06 ` Thomas S. Dye
2014-08-09 8:54 ` Rainer M Krug
2014-08-16 5:05 ` Aaron Ecay
2014-08-16 18:50 ` Charles C. Berry
2014-08-16 20:58 ` Aaron Ecay
2014-08-17 6:03 ` Achim Gratz
2014-08-19 0:13 ` Aaron Ecay
2014-08-19 5:36 ` Achim Gratz
2014-08-23 8:32 ` Aaron Ecay
2014-08-23 9:24 ` Andreas Kiermeier
2014-08-23 17:10 ` Aaron Ecay
2014-08-23 18:35 ` Thomas S. Dye
2014-08-23 19:37 ` Ista Zahn
2014-08-24 0:10 ` Charles C. Berry [this message]
2014-08-28 5:24 ` Aaron Ecay
2014-09-01 5:00 ` Aaron Ecay
2014-09-01 16:08 ` Charles C. Berry
2014-08-09 8:48 ` Rainer M Krug
2014-08-06 1:11 ` Andreas Kiermeier
2014-08-06 2:21 ` Charles C. Berry
2014-08-06 3:24 ` Aaron Ecay
2014-08-06 15:59 ` Charles C. Berry
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.OSX.2.00.1408231558290.2651@charles-berrys-macbook.local \
--to=ccberry@ucsd.edu \
--cc=aaronecay@gmail.com \
--cc=emacs-orgmode@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).