[babel][PATCHES] ob-R patches for review

[babel][PATCHES] ob-R patches for review

[Rainer M Krug]

[-- Attachment #1.1: Type: text/plain, Size: 625 bytes --]

Hi

Attached please find seven patches for review to implement the storing
of org variables in their own environment and to make the org-issued R
code look nicer in the R session.

Thanks,

Rainer

-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-ob-R.el-Write-org-variables-into-own-R-environment.patch --]
[-- Type: text/x-patch, Size: 3528 bytes --]

From b1199e03758f42fde2f75f02a4c18cd71fac5a03 Mon Sep 17 00:00:00 2001
From: "Rainer M. Krug" <R.M.Krug@gmail.com>
Date: Thu, 20 Mar 2014 11:35:44 +0100
Subject: [PATCH 1/8] ob-R.el: Write org variables into own R environment

* lisp/ob-R.el (org-babel-expand-body:R): Create empty environment
  called `org' in R before adding variables and afterwards lock it and
  add it to the R search path.
  (org-babel-variable-assignments:R): Assign variables into own
  `org' environment in R instead of .GlobalEnv

These patch implements the writing of org variables into a separate R
environment and attaches it to the search path.  For the usage of these
variables, nothing changes in R, but:

1) The org variables are now grouped and can be seen via `ls(org)' in
R and are not shown anymore in the .GlobalEnv when using `ls()'
2) As the environment `org' and all bindings are locked, the variables
can not be accidentally deleted. They can be overwritten, but they can
be restored by simply deleting the variable in R or by using
`org$VARIABLE' instead of `VARIABLE'
3) All variables can be saved by simply calling `save(org, FILENAME'
in R which makes it possible to store all variable definitions for
tangling in one file.
---
 lisp/ob-R.el | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 62aa7f2..82971de 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -103,8 +103,12 @@ this variable.")
             (append
              (when (cdr (assoc :prologue params))
                (list (cdr (assoc :prologue params))))
-             (org-babel-variable-assignments:R params)
-             (list body)
+	     '("try(detach(org), silent=TRUE)")
+             '("org <- new.env()")
+	     (org-babel-variable-assignments:R params)
+	     '("lockEnvironment(org)")
+	     '("attach(org)")
+	     (list body)
              (when (cdr (assoc :epilogue params))
                (list (cdr (assoc :epilogue params)))))))
        (if graphics-file
@@ -203,20 +207,9 @@ This function is called by `org-babel-execute-src-block'."
 			  "TRUE" "FALSE"))
 	      (row-names (if rownames-p "1" "NULL")))
 	  (if (= max min)
-	      (format "%s <- read.table(\"%s\",
-                      header=%s,
-                      row.names=%s,
-                      sep=\"\\t\",
-                      as.is=TRUE)" name file header row-names)
-	    (format "%s <- read.table(\"%s\",
-                   header=%s,
-                   row.names=%s,
-                   sep=\"\\t\",
-                   as.is=TRUE,
-                   fill=TRUE,
-                   col.names = paste(\"V\", seq_len(%d), sep =\"\"))"
-		    name file header row-names max))))
-    (format "%s <- %s" name (org-babel-R-quote-tsv-field value))))
+	      (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE ), envir = org ); lockBinding('%s', org)" name file header row-names name)
+	    (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE, fill=TRUE, col.names = paste(\"V\", seq_len(%d), sep =\"\") ), envir = org ); lockBinding('%s', org)" name file header row-names max name))))
+    (format "assign('%s', %s, envir = org); lockBinding('%s', org)" name (org-babel-R-quote-tsv-field value) name)))
 
 (defvar ess-ask-for-ess-directory) ; dynamically scoped
 (defun org-babel-R-initiate-session (session params)
-- 
1.8.5.2 (Apple Git-48)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: 0002-Add-saving-of-org-variable-document.patch --]
[-- Type: text/x-patch, Size: 874 bytes --]

From 9b1aed4c855fd54caa7b91e316b26ef7a813b7b0 Mon Sep 17 00:00:00 2001
From: "Rainer M. Krug" <R.M.Krug@gmail.com>
Date: Wed, 26 Mar 2014 16:53:29 +0100
Subject: [PATCH 2/8] Add saving of org variable document

* lisp/ob-R.el (org-babel-expand-body:R): add R command to save `org'
  environment after variables are stored and locked.
---
 lisp/ob-R.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 82971de..af90e68 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -108,6 +108,7 @@ this variable.")
 	     (org-babel-variable-assignments:R params)
 	     '("lockEnvironment(org)")
 	     '("attach(org)")
+	     '("save(org, file='orgVARIABLES.RData')")
 	     (list body)
              (when (cdr (assoc :epilogue params))
                (list (cdr (assoc :epilogue params)))))))
-- 
1.8.5.2 (Apple Git-48)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.4: 0003-Change-R-environment-name-from-org-to-org_variables_.patch --]
[-- Type: text/x-patch, Size: 2682 bytes --]

From 27a9daaf3b94c23b437d55c5d1e3d282139ced22 Mon Sep 17 00:00:00 2001
From: "Rainer M. Krug" <R.M.Krug@gmail.com>
Date: Wed, 26 Mar 2014 17:09:15 +0100
Subject: [PATCH 3/8] Change R environment name from org to org_variables_

---
 lisp/ob-R.el | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index af90e68..b51e582 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -103,12 +103,12 @@ this variable.")
             (append
              (when (cdr (assoc :prologue params))
                (list (cdr (assoc :prologue params))))
-	     '("try(detach(org), silent=TRUE)")
-             '("org <- new.env()")
+	     '("try(detach(org_variables_), silent=TRUE)")
+             '("org_variables_ <- new.env()")
 	     (org-babel-variable-assignments:R params)
-	     '("lockEnvironment(org)")
-	     '("attach(org)")
-	     '("save(org, file='orgVARIABLES.RData')")
+	     '("lockEnvironment(org_variables_)")
+	     '("attach(org_variables_)")
+	     '("save(org_variables_, file='org_variables.RData')")
 	     (list body)
              (when (cdr (assoc :epilogue params))
                (list (cdr (assoc :epilogue params)))))))
@@ -208,9 +208,9 @@ This function is called by `org-babel-execute-src-block'."
 			  "TRUE" "FALSE"))
 	      (row-names (if rownames-p "1" "NULL")))
 	  (if (= max min)
-	      (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE ), envir = org ); lockBinding('%s', org)" name file header row-names name)
-	    (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE, fill=TRUE, col.names = paste(\"V\", seq_len(%d), sep =\"\") ), envir = org ); lockBinding('%s', org)" name file header row-names max name))))
-    (format "assign('%s', %s, envir = org); lockBinding('%s', org)" name (org-babel-R-quote-tsv-field value) name)))
+	      (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE ), envir = org_variables_ ); lockBinding('%s', org_variables_)" name file header row-names name)
+	    (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE, fill=TRUE, col.names = paste(\"V\", seq_len(%d), sep =\"\") ), envir = org_variables_ ); lockBinding('%s', org_variables_)" name file header row-names max name))))
+    (format "assign('%s', %s, envir = org_variables_); lockBinding('%s', org_variables_)" name (org-babel-R-quote-tsv-field value) name)))
 
 (defvar ess-ask-for-ess-directory) ; dynamically scoped
 (defun org-babel-R-initiate-session (session params)
-- 
1.8.5.2 (Apple Git-48)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.5: 0004-Rename-org_variables-to-.org_variables.patch --]
[-- Type: text/x-patch, Size: 3060 bytes --]

From e8c977e0b38eb7eb64ed29f55bd985ca2e135c4c Mon Sep 17 00:00:00 2001
From: "Rainer M. Krug" <R.M.Krug@gmail.com>
Date: Fri, 28 Mar 2014 12:13:53 +0100
Subject: [PATCH 4/8] Rename `org_variables' to `.org_variables'

* lisp/ob-R.el (org-babel-R-assign-elisp, org-babel-expand-body:R):
  Change name of R environment from `org_variables' to
  `.org_variables'. The `.' makes the environment hidden in R and it
  can not be directly deleted in R by using `rm(list=ls()'
---
 lisp/ob-R.el | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index b51e582..214c14d 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -103,12 +103,12 @@ this variable.")
             (append
              (when (cdr (assoc :prologue params))
                (list (cdr (assoc :prologue params))))
-	     '("try(detach(org_variables_), silent=TRUE)")
-             '("org_variables_ <- new.env()")
+	     '("try(detach(.org_variables_), silent=TRUE)")
+             '(".org_variables_ <- new.env()")
 	     (org-babel-variable-assignments:R params)
-	     '("lockEnvironment(org_variables_)")
-	     '("attach(org_variables_)")
-	     '("save(org_variables_, file='org_variables.RData')")
+	     '("lockEnvironment(.org_variables_)")
+	     '("attach(.org_variables_)")
+	     '("save(.org_variables_, file='org_variables.RData')")
 	     (list body)
              (when (cdr (assoc :epilogue params))
                (list (cdr (assoc :epilogue params)))))))
@@ -208,9 +208,9 @@ This function is called by `org-babel-execute-src-block'."
 			  "TRUE" "FALSE"))
 	      (row-names (if rownames-p "1" "NULL")))
 	  (if (= max min)
-	      (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE ), envir = org_variables_ ); lockBinding('%s', org_variables_)" name file header row-names name)
-	    (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE, fill=TRUE, col.names = paste(\"V\", seq_len(%d), sep =\"\") ), envir = org_variables_ ); lockBinding('%s', org_variables_)" name file header row-names max name))))
-    (format "assign('%s', %s, envir = org_variables_); lockBinding('%s', org_variables_)" name (org-babel-R-quote-tsv-field value) name)))
+	      (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE ), envir = .org_variables_ ); lockBinding('%s', .org_variables_)" name file header row-names name)
+	    (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE, fill=TRUE, col.names = paste(\"V\", seq_len(%d), sep =\"\") ), envir = .org_variables_ ); lockBinding('%s', .org_variables_)" name file header row-names max name))))
+    (format "assign('%s', %s, envir = .org_variables_); lockBinding('%s', .org_variables_)" name (org-babel-R-quote-tsv-field value) name)))
 
 (defvar ess-ask-for-ess-directory) ; dynamically scoped
 (defun org-babel-R-initiate-session (session params)
-- 
1.8.5.2 (Apple Git-48)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.6: 0005-Adds-visual-separators.patch --]
[-- Type: text/x-patch, Size: 1421 bytes --]

From ba532e45ba7fe373effbeef3781e744b33f532a4 Mon Sep 17 00:00:00 2001
From: "Rainer M. Krug" <R.M.Krug@gmail.com>
Date: Mon, 31 Mar 2014 16:57:14 +0200
Subject: [PATCH 5/8] Adds visual separators

* ob-R.el (org-babel-expand-body:R): Add visual separators in R code
  to separate the automatically added code from the code from the code blocks.
---
 lisp/ob-R.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 214c14d..3534580 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -103,12 +103,16 @@ this variable.")
             (append
              (when (cdr (assoc :prologue params))
                (list (cdr (assoc :prologue params))))
+	     '("######################################################################")
+	     '("## Beginning org variable transfer")
 	     '("try(detach(.org_variables_), silent=TRUE)")
              '(".org_variables_ <- new.env()")
 	     (org-babel-variable-assignments:R params)
 	     '("lockEnvironment(.org_variables_)")
 	     '("attach(.org_variables_)")
 	     '("save(.org_variables_, file='org_variables.RData')")
+	     '("## end org variable transfer")
+	     '("######################################################################")
 	     (list body)
              (when (cdr (assoc :epilogue params))
                (list (cdr (assoc :epilogue params)))))))
-- 
1.8.5.2 (Apple Git-48)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.7: 0006-Make-R-code-look-nicer-in-R.patch --]
[-- Type: text/x-patch, Size: 3013 bytes --]

From b3ddb3bf0bb327187acdd898a6173d22ceefe5fb Mon Sep 17 00:00:00 2001
From: "Rainer M. Krug" <R.M.Krug@gmail.com>
Date: Mon, 7 Apr 2014 10:52:00 +0200
Subject: [PATCH 6/8] Make R code look nicer in R

* lisp/ob-R.el (org-babel-expand-body:R): Added Space to the R code
  blocks to make them easier identifiable in R.
---
 lisp/ob-R.el | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 3534580..5edbed9 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -105,12 +105,12 @@ this variable.")
                (list (cdr (assoc :prologue params))))
 	     '("######################################################################")
 	     '("## Beginning org variable transfer")
-	     '("try(detach(.org_variables_), silent=TRUE)")
-             '(".org_variables_ <- new.env()")
+	     '("  try(detach(.org_variables_), silent=TRUE)")
+             '("  .org_variables_ <- new.env()")
 	     (org-babel-variable-assignments:R params)
-	     '("lockEnvironment(.org_variables_)")
-	     '("attach(.org_variables_)")
-	     '("save(.org_variables_, file='org_variables.RData')")
+	     '("  lockEnvironment(.org_variables_)")
+	     '("  attach(.org_variables_)")
+	     '("  save(.org_variables_, file='org_variables.RData')")
 	     '("## end org variable transfer")
 	     '("######################################################################")
 	     (list body)
@@ -212,9 +212,9 @@ This function is called by `org-babel-execute-src-block'."
 			  "TRUE" "FALSE"))
 	      (row-names (if rownames-p "1" "NULL")))
 	  (if (= max min)
-	      (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE ), envir = .org_variables_ ); lockBinding('%s', .org_variables_)" name file header row-names name)
-	    (format "assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE, fill=TRUE, col.names = paste(\"V\", seq_len(%d), sep =\"\") ), envir = .org_variables_ ); lockBinding('%s', .org_variables_)" name file header row-names max name))))
-    (format "assign('%s', %s, envir = .org_variables_); lockBinding('%s', .org_variables_)" name (org-babel-R-quote-tsv-field value) name)))
+	      (format "    assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE ), envir = .org_variables_ ); lockBinding('%s', .org_variables_)" name file header row-names name)
+	    (format "    assign( '%s', read.table(\"%s\", header=%s, row.names=%s, sep=\"\\t\", as.is=TRUE, fill=TRUE, col.names = paste(\"V\", seq_len(%d), sep =\"\") ), envir = .org_variables_ ); lockBinding('%s', .org_variables_)" name file header row-names max name))))
+    (format "    assign('%s', %s, envir = .org_variables_); lockBinding('%s', .org_variables_)" name (org-babel-R-quote-tsv-field value) name)))
 
 (defvar ess-ask-for-ess-directory) ; dynamically scoped
 (defun org-babel-R-initiate-session (session params)
-- 
1.8.5.2 (Apple Git-48)


[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Re: [babel][PATCHES] ob-R patches for review

[Charles Berry]

Rainer M Krug <Rainer <at> krugs.de> writes:

> 
> Hi
> 
> Attached please find seven patches for review to implement the storing
> of org variables in their own environment and to make the org-issued R
> code look nicer in the R session.


Rainer,


I have suggestions and a concern.

I suggest that you look at how ESS handles R objects and constructs
calls in elisp to be executed in an R session.

It uses a package and its NAMESPACE to provide that functionality and
store objects. That makes the elisp interface a lot cleaner and keeps
ESS variables out of the users way. The package is found at <ESS>/etc/ESSR/.

I also suggest that you introduce a customization variable to
allow a user to turn off the functionality you have created.

My concern is that you are injecting code into the R user session or
script that the user may not want. If I already have an R object named
'org' your code will break my code.

Further, all of this is hard coded, so I can't change the
variable/file names.

HTH,

Chuck

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

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

Charles Berry <ccberry@ucsd.edu> writes:

> Rainer M Krug <Rainer <at> krugs.de> writes:
>
>> 
>> Hi
>> 
>> Attached please find seven patches for review to implement the storing
>> of org variables in their own environment and to make the org-issued R
>> code look nicer in the R session.
>
>
> Rainer,
>
>
> I have suggestions and a concern.
>
> I suggest that you look at how ESS handles R objects and constructs
> calls in elisp to be executed in an R session.
>
> It uses a package and its NAMESPACE to provide that functionality and
> store objects. That makes the elisp interface a lot cleaner and keeps
> ESS variables out of the users way. The package is found at <ESS>/etc/ESSR/.

That is effectively what I am doing as well, only that I am not using a
package but an environment and add it to the search path. 

By doing this, the org variables are stored in this environment and do
not overwrite existing R objects by the same name. If objects by the
same name as the org variables are existing in the global R environment,
they are still found first, but variables transferred from org can
*always* be accessed by using

,----
| .org_variables_$VARIABLE_NAME
`----

in R. As said, existing VARIABLE_NAME objects in R are *not* impacted
on.

If you use only

,----
| VARIABLE_NAME
`----

in R it returns the first object in the R search path.

So the only variable which can be overwritten when this patch is used is

,----
| .org_variables_
`----

which, I think, is quite unlikely to exist.

Concerning package: I like the idea and thought about it with the aim of
putting all R code into a package, to make it easier to maintain the
code and to customize it. The org variables can then be injected in
this package. But the problem of name clashes between user defined
variables and the name of the namespace / environment of the package is
the same - possible higher, as a package name (and therefore the
namespace and environment) should have meaningful names.

>
> I also suggest that you introduce a customization variable to
> allow a user to turn off the functionality you have created.

I don't think this is necessary as the behavior for the user does not
change at all, only that it becomes safer to use org variables in R (see
above).

>
> My concern is that you are injecting code into the R user session or
> script that the user may not want. If I already have an R object named
> 'org' your code will break my code.

Please see my later patch, which renames this to ".org_variables_" which
sounds like an unlikely name to exist.

>
> Further, all of this is hard coded, so I can't change the
> variable/file names.

OK - this can be added as a variable, and in regards to the file name
this makes sense. For the moment I would leave it as it is and discuss
what would be the useful approach, as there are some problems with the
usefulness of saving the variables - imagining different variables
passed to different code blocks. One option would be to have a variable,
whose default is NIL, which contains the file name:

- NIL :: no saving
- string :: saving under the name given



>
> HTH,

Yes definitely - discussions more then welcome,

Rainer

>
> Chuck
>
>
>
>
>

-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Re: [babel][PATCHES] ob-R patches for review

[Charles C. Berry]

On Wed, 30 Apr 2014, Rainer M Krug wrote:

> Charles Berry <ccberry@ucsd.edu> writes:
>
>> Rainer M Krug <Rainer <at> krugs.de> writes:
>>
>>>
>>> Hi
>>>
>>> Attached please find seven patches for review to implement the storing
>>> of org variables in their own environment and to make the org-issued R
>>> code look nicer in the R session.
>>
>>
>> Rainer,
>>
>>
>> I have suggestions and a concern.
>>
>> I suggest [...]


> That is effectively what I am doing as well, only that I am not using a
> package but an environment and add it to the search path.
>

[...]

OK. I did not study your patches closely enough. Sorry.


>
>>
>> I also suggest that you introduce a customization variable to
>> allow a user to turn off the functionality you have created.
>
> I don't think this is necessary as the behavior for the user does not
> change at all, only that it becomes safer to use org variables in R (see
> above).
>

All you have to do is add this:

(defvar org-babel-R-assign-elisp-function 'org-babel-R-assign-elisp
   "Name or definition of function to handle `:var name=value'
header args."
   )

and change one line in org-babel-variable-assignments:R from

     (org-babel-R-assign-elisp 
to

    (funcall org-babel-R-assign-elisp-function

and the user can provide her own elisp assignment function.

This gives users who want special behavior like creating something
other than a data.frame the option of providing their own function.


Best,


Chuck

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

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

"Charles C. Berry" <ccberry@ucsd.edu> writes:

> On Wed, 30 Apr 2014, Rainer M Krug wrote:
>
>> Charles Berry <ccberry@ucsd.edu> writes:
>>
>>> Rainer M Krug <Rainer <at> krugs.de> writes:
>>>
>>>>
>>>> Hi
>>>>
>>>> Attached please find seven patches for review to implement the storing
>>>> of org variables in their own environment and to make the org-issued R
>>>> code look nicer in the R session.
>>>
>>>
>>> Rainer,
>>>
>>>
>>> I have suggestions and a concern.
>>>
>>> I suggest [...]
>
>
>> That is effectively what I am doing as well, only that I am not using a
>> package but an environment and add it to the search path.
>>
>
> [...]
>
> OK. I did not study your patches closely enough. Sorry.

No problem.

>
>
>>
>>>
>>> I also suggest that you introduce a customization variable to
>>> allow a user to turn off the functionality you have created.
>>
>> I don't think this is necessary as the behavior for the user does not
>> change at all, only that it becomes safer to use org variables in R (see
>> above).
>>
>
> All you have to do is add this:
>
> (defvar org-babel-R-assign-elisp-function 'org-babel-R-assign-elisp
>   "Name or definition of function to handle `:var name=value'
> header args."
>   )
>
> and change one line in org-babel-variable-assignments:R from
>
>     (org-babel-R-assign-elisp to
>
>    (funcall org-babel-R-assign-elisp-function
>
> and the user can provide her own elisp assignment function.
>
> This gives users who want special behavior like creating something
> other than a data.frame the option of providing their own function.

This assumes, that the user knows elisp. For many customizations this is
necessary, but I would prefer a system where the user only has to
provide an R function which will be used. This offers less
customizability, but this would make it possible to use R to do the
customization. To write a new org-babel-R-assign-elisp-function would be
quite a challenge for an R programmer (like me...).

Thanks,

Rainer



>
>
> Best,
>
>
> Chuck

-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Re: [babel][PATCHES] ob-R patches for review

[Eric Schulte]

Rainer M Krug <Rainer@krugs.de> writes:

> Hi
>
> Attached please find seven patches for review to implement the storing
> of org variables in their own environment and to make the org-issued R
> code look nicer in the R session.
>
> Thanks,
>
> Rainer

Hi Rainer,

Thanks for these patches.  I don't have the R experience to review or
maintain them, but I'm happy to apply them.

I missed some previous discussion in this thread.  Are these patches
ready to be applied as is?

Thanks,

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

Re: [babel][PATCHES] ob-R patches for review

[Charles Berry]

Eric Schulte <schulte.eric <at> gmail.com> writes:

> 
> Rainer M Krug <Rainer <at> krugs.de> writes:
> 
> > Hi
> >
> > Attached please find seven patches for review to implement the storing
> > of org variables in their own environment and to make the org-issued R
> > code look nicer in the R session.
> >
> > Thanks,
> >
> > Rainer
> 
> Hi Rainer,
> 
> Thanks for these patches.  I don't have the R experience to review or
> maintain them, but I'm happy to apply them.
> 
> I missed some previous discussion in this thread.  Are these patches
> ready to be applied as is?
>


IMO, the patches hard coded behaviors that would better be customizable
and optional. 

Rainer and I had some back and forth about this -- see the thread.

Best,

Chuck

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

> Le 7 mai 2014 à 12:27, Eric Schulte <schulte.eric@gmail.com> a écrit :
> 
> Rainer M Krug <Rainer@krugs.de> writes:
> 
>> Hi
>> 
>> Attached please find seven patches for review to implement the storing
>> of org variables in their own environment and to make the org-issued R
>> code look nicer in the R session.
>> 
>> Thanks,
>> 
>> Rainer
> 
> Hi Rainer,
> 
> Thanks for these patches.  I don't have the R experience to review or
> maintain them, but I'm happy to apply them.
> 
> I missed some previous discussion in this thread.  Are these patches
> ready to be applied as is?

I'll look at it again tomorrow and let you know as I made some changes since then. Do you prefer one patch to several?

Cheers,

Rainer

> 
> Thanks,
> 
> -- 
> Eric Schulte
> https://cs.unm.edu/~eschulte
> PGP: 0x614CA05D

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

Envoyé de mon iPhone

> Le 8 mai 2014 à 04:26, Charles Berry <ccberry@ucsd.edu> a écrit :
> 
> Eric Schulte <schulte.eric <at> gmail.com> writes:
> 
>> 
>> Rainer M Krug <Rainer <at> krugs.de> writes:
>> 
>>> Hi
>>> 
>>> Attached please find seven patches for review to implement the storing
>>> of org variables in their own environment and to make the org-issued R
>>> code look nicer in the R session.
>>> 
>>> Thanks,
>>> 
>>> Rainer
>> 
>> Hi Rainer,
>> 
>> Thanks for these patches.  I don't have the R experience to review or
>> maintain them, but I'm happy to apply them.
>> 
>> I missed some previous discussion in this thread.  Are these patches
>> ready to be applied as is?
> 
> 
> IMO, the patches hard coded behaviors that would better be customizable
> and optional. 

I'll give feedback tomorrow and let you know about customization of the name of the environment. The general behavior of storing the variables in an environment should not be customizable as it is

1) safer then the behavior of storing each variable separately
2) no changes for the user are introduced 

Cheers

Rainer

> 
> Rainer and I had some back and forth about this -- see the thread.
> 
> Best,
> 
> Chuck
> 
> 
> 

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

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

Rainer M Krug <r.m.krug@gmail.com> writes:

> Envoyé de mon iPhone
>
>> Le 8 mai 2014 à 04:26, Charles Berry <ccberry@ucsd.edu> a écrit :
>> 
>> Eric Schulte <schulte.eric <at> gmail.com> writes:
>> 
>>> 
>>> Rainer M Krug <Rainer <at> krugs.de> writes:
>>> 
>>>> Hi
>>>> 
>>>> Attached please find seven patches for review to implement the storing
>>>> of org variables in their own environment and to make the org-issued R
>>>> code look nicer in the R session.
>>>> 
>>>> Thanks,
>>>> 
>>>> Rainer
>>> 
>>> Hi Rainer,
>>> 
>>> Thanks for these patches.  I don't have the R experience to review or
>>> maintain them, but I'm happy to apply them.
>>> 
>>> I missed some previous discussion in this thread.  Are these patches
>>> ready to be applied as is?
>> 
>> 
>> IMO, the patches hard coded behaviors that would better be customizable
>> and optional. 
>
> I'll give feedback tomorrow and let you know about customization of
> the name of the environment. The general behavior of storing the
> variables in an environment should not be customizable as it is
>
> 1) safer then the behavior of storing each variable separately
> 2) no changes for the user are introduced 

OK. Concerning customization options:

There are a few points where customizations could be introduced:

1) transfer of variables from org to R, i.e. old behavior (variables in
.GlobalEnv) or new behavior (in own environment (.org_variables_)) or
own defined function

2) name of new environment

3) name of file into which to save the  variables (org_variables.RData)

Some comments to each:

1) As outlined above, I see neither changes in behavior for the user
nor disadvantages of the new behavior. As only one object is created in
R (.org_variables_) instead of one for each variable, the chances of
name clashes are much smaller. As the name of the variable starts with a
dot (.), ends with an underscore (_), it is very unlikely that there is
a nameclash in existing scripts (but you are right, one does not know).
So I don't think, that it is necessary to include an option for
disabling the old behavior. 

Concerning defining an own function for data transfer, I am thinking of
putting the variable transfer into an R function which can then be
customized from R. My reasoning is that users using this feature are
more likely to be fluent in R then in lisp, so more able to change these
functions in R. In org, the whole R code would then simply replaced with
one function call. These functions would reside in a new environment (or
in the ESSR environment as offered by Vitalie).

In addition, as it was not asked before to have this function
customizable, I don't think there would be a large need for it.

2) I don't think name clashes are likely, so I don't see a real need to
have the name of the org environment in R configurable. Especially when
using these variables in R, one can always use
.org_variables_::VARIABLENAME to access the original value. If the
environment name is customizable, this will be different between
customizations and not that easily reproducible between
org-installations.

It would be easy to configure it, but I think it is rather a
disadvantage and would make the code (slightly) more complex.

3) The name into which the variables are saved could be configurable,
but again, I do not think this is such an issue. The saved environment
is anyway only of limited usage when the variables in org are not
defined file wide but per source block / tree. I am actually thinking of
removing this saving, although it is quite useful to make tangled code
usable on a non-org system when org variables are used - comments?

So please let me know if you see the need of customization and for which aspect.

>
> Cheers
>
> Rainer
>
>> 
>> Rainer and I had some back and forth about this -- see the thread.
>> 
>> Best,
>> 
>> Chuck
>> 
>> 
>> 

-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

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

Rainer M Krug <Rainer@krugs.de> writes:

> Rainer M Krug <r.m.krug@gmail.com> writes:
>
>> Envoyé de mon iPhone
>>
>>> Le 8 mai 2014 à 04:26, Charles Berry <ccberry@ucsd.edu> a écrit :
>>> 
>>> Eric Schulte <schulte.eric <at> gmail.com> writes:
>>> 
>>>> 
>>>> Rainer M Krug <Rainer <at> krugs.de> writes:
>>>> 
>>>>> Hi
>>>>> 
>>>>> Attached please find seven patches for review to implement the storing
>>>>> of org variables in their own environment and to make the org-issued R
>>>>> code look nicer in the R session.
>>>>> 
>>>>> Thanks,
>>>>> 
>>>>> Rainer
>>>> 
>>>> Hi Rainer,
>>>> 
>>>> Thanks for these patches.  I don't have the R experience to review or
>>>> maintain them, but I'm happy to apply them.
>>>> 
>>>> I missed some previous discussion in this thread.  Are these patches
>>>> ready to be applied as is?
>>> 
>>> 
>>> IMO, the patches hard coded behaviors that would better be customizable
>>> and optional. 
>>
>> I'll give feedback tomorrow and let you know about customization of
>> the name of the environment. The general behavior of storing the
>> variables in an environment should not be customizable as it is
>>
>> 1) safer then the behavior of storing each variable separately
>> 2) no changes for the user are introduced 
>
> OK. Concerning customization options:
>
> There are a few points where customizations could be introduced:
>
> 1) transfer of variables from org to R, i.e. old behavior (variables in
> .GlobalEnv) or new behavior (in own environment (.org_variables_)) or
> own defined function
>
> 2) name of new environment
>
> 3) name of file into which to save the  variables (org_variables.RData)
>
> Some comments to each:
>
> 1) As outlined above, I see neither changes in behavior for the user
> nor disadvantages of the new behavior. As only one object is created in
> R (.org_variables_) instead of one for each variable, the chances of
> name clashes are much smaller. As the name of the variable starts with a
> dot (.), ends with an underscore (_), it is very unlikely that there is
> a nameclash in existing scripts (but you are right, one does not know).
> So I don't think, that it is necessary to include an option for
> disabling the old behavior. 
>
> Concerning defining an own function for data transfer, I am thinking of
> putting the variable transfer into an R function which can then be
> customized from R. My reasoning is that users using this feature are
> more likely to be fluent in R then in lisp, so more able to change these
> functions in R. In org, the whole R code would then simply replaced with
> one function call. These functions would reside in a new environment (or
> in the ESSR environment as offered by Vitalie).
>
> In addition, as it was not asked before to have this function
> customizable, I don't think there would be a large need for it.
>
> 2) I don't think name clashes are likely, so I don't see a real need to
> have the name of the org environment in R configurable. Especially when
> using these variables in R, one can always use
> .org_variables_::VARIABLENAME to access the original value. If the
> environment name is customizable, this will be different between
> customizations and not that easily reproducible between
> org-installations.
>
> It would be easy to configure it, but I think it is rather a
> disadvantage and would make the code (slightly) more complex.
>
> 3) The name into which the variables are saved could be configurable,
> but again, I do not think this is such an issue. The saved environment
> is anyway only of limited usage when the variables in org are not
> defined file wide but per source block / tree. I am actually thinking of
> removing this saving, although it is quite useful to make tangled code
> usable on a non-org system when org variables are used - comments?
>
> So please let me know if you see the need of customization and for
> which aspect.

I started looking into ESS in more detail and I got some useful ideas. 

I am including them at the moment in org, so please do not yet apply the
patches.

Cheers,

Rainer

>
>>
>> Cheers
>>
>> Rainer
>>
>>> 
>>> Rainer and I had some back and forth about this -- see the thread.
>>> 
>>> Best,
>>> 
>>> Chuck
>>> 
>>> 
>>> 

-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Re: [babel][PATCHES] ob-R patches for review

[Bastien]

Hi Rainer,

Rainer M Krug <r.m.krug@gmail.com> writes:

> I'll look at it again tomorrow and let you know as I made some changes
> since then. Do you prefer one patch to several?

Up to Eric's taste -- but in general I think a series of patches
is better, it allows you to isolate and fix conflicts more easily.

-- 
 Bastien

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

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

Bastien <bzg@gnu.org> writes:

> Hi Rainer,
>
> Rainer M Krug <r.m.krug@gmail.com> writes:
>
>> I'll look at it again tomorrow and let you know as I made some changes
>> since then. Do you prefer one patch to several?
>
> Up to Eric's taste -- but in general I think a series of patches
> is better, it allows you to isolate and fix conflicts more easily.

Thanks

Rainer

-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Re: [babel][PATCHES] ob-R patches for review

[Eric Schulte]

Rainer M Krug <Rainer@krugs.de> writes:

> Bastien <bzg@gnu.org> writes:
>
>> Hi Rainer,
>>
>> Rainer M Krug <r.m.krug@gmail.com> writes:
>>
>>> I'll look at it again tomorrow and let you know as I made some changes
>>> since then. Do you prefer one patch to several?
>>
>> Up to Eric's taste -- but in general I think a series of patches
>> is better, it allows you to isolate and fix conflicts more easily.
>

I agree, multiple patches make future maintenance easier.

>> I missed some previous discussion in this thread.  Are these patches
>> ready to be applied as is?
>>
>
>
> IMO, the patches hard coded behaviors that would better be customizable
> and optional. 
>
> Rainer and I had some back and forth about this -- see the thread.

With respect to these points, I'm inclined to agree with Charles in the
following.

> All you have to do is add this:
>
> (defvar org-babel-R-assign-elisp-function 'org-babel-R-assign-elisp
>   "Name or definition of function to handle `:var name=value'
> header args."
>   )
>
> and change one line in org-babel-variable-assignments:R from
>
>     (org-babel-R-assign-elisp to
>
>    (funcall org-babel-R-assign-elisp-function
>
> and the user can provide her own elisp assignment function.
>
> This gives users who want special behavior like creating something
> other than a data.frame the option of providing their own function.

Would such a customization variable be difficult to add to your patches?
If not would you mind submitting a version of the patches split into
multiple commits with as much of the hard-coded R code as feasible
placed into customizable variables along the lines of the
`org-babel-R-assign-elisp-function' variable suggested by Charles.  One
lesson I've certainly learned from the Org-mode mailing list is that you
can't anticipate all of the ways that your code will be used, so
up-front customizability generally pays off.

Thanks,
Eric

>
> Thanks
>
> Rainer

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

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

Eric Schulte <schulte.eric@gmail.com> writes:

> Rainer M Krug <Rainer@krugs.de> writes:
>
>> Bastien <bzg@gnu.org> writes:
>>
>>> Hi Rainer,
>>>
>>> Rainer M Krug <r.m.krug@gmail.com> writes:
>>>
>>>> I'll look at it again tomorrow and let you know as I made some changes
>>>> since then. Do you prefer one patch to several?
>>>
>>> Up to Eric's taste -- but in general I think a series of patches
>>> is better, it allows you to isolate and fix conflicts more easily.
>>
>
> I agree, multiple patches make future maintenance easier.
>

OK - I'll do so.

A little bit off-topic, is there a "git way" of splitting one patch into
several patches, if it was a single commit?

>>> I missed some previous discussion in this thread.  Are these patches
>>> ready to be applied as is?
>>>
>>
>>
>> IMO, the patches hard coded behaviors that would better be customizable
>> and optional. 
>>
>> Rainer and I had some back and forth about this -- see the thread.
>
> With respect to these points, I'm inclined to agree with Charles in the
> following.

OK - see further comments below.

>
>> All you have to do is add this:
>>
>> (defvar org-babel-R-assign-elisp-function 'org-babel-R-assign-elisp
>>   "Name or definition of function to handle `:var name=value'
>> header args."
>>   )
>>
>> and change one line in org-babel-variable-assignments:R from
>>
>>     (org-babel-R-assign-elisp to
>>
>>    (funcall org-babel-R-assign-elisp-function
>>
>> and the user can provide her own elisp assignment function.
>>
>> This gives users who want special behavior like creating something
>> other than a data.frame the option of providing their own function.
>
> Would such a customization variable be difficult to add to your patches?

I don't think so - I'll look into it.

> If not would you mind submitting a version of the patches split into
> multiple commits with as much of the hard-coded R code as feasible
> placed into customizable variables along the lines of the
> `org-babel-R-assign-elisp-function' variable suggested by Charles.  

I am thinking of actually not providing the R code in org-variables, but
to put them into R files and to source them. By doing this, the
customization could be done in R, which will be much easier for R users
then to customize emacs variables.

These would be sourced and stored into an environment "org:functions",
using the same approach as ESS is using to store functions into an
environment "ESSR". I would then put the variables transfered into
"org:variables". These environments would only exist in the search path,
and not overwrite any user set objects in R.

As it needs to be sourced for each R process once, the right place would
be in  org-babel-R-initiate-session - correct?

What would be the best place to put these R files? 

> One lesson I've certainly learned from the Org-mode mailing list is
> that you can't anticipate all of the ways that your code will be used,
> so up-front customizability generally pays off.

OK - point taken - and I am definitely one of those users who thinks
about unusual usages of certain features.

Cheers,

Rainer

>
> Thanks,
> Eric
>
>>
>> Thanks
>>
>> Rainer

-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Re: [babel][PATCHES] ob-R patches for review

[Suvayu Ali]

On Mon, May 12, 2014 at 10:33:48AM +0200, Rainer M Krug wrote:
> Eric Schulte <schulte.eric@gmail.com> writes:
> 
> > Rainer M Krug <Rainer@krugs.de> writes:
> >
> >> Bastien <bzg@gnu.org> writes:
> >>
> >>> Hi Rainer,
> >>>
> >>> Rainer M Krug <r.m.krug@gmail.com> writes:
> >>>
> >>>> I'll look at it again tomorrow and let you know as I made some changes
> >>>> since then. Do you prefer one patch to several?
> >>>
> >>> Up to Eric's taste -- but in general I think a series of patches
> >>> is better, it allows you to isolate and fix conflicts more easily.
> >>
> >
> > I agree, multiple patches make future maintenance easier.
> >
> 
> OK - I'll do so.
> 
> A little bit off-topic, is there a "git way" of splitting one patch into
> several patches, if it was a single commit?

Do an interactive rebase, and amend.

Say this is the commit graph:

  A---B---C---D

You want to split B.  Then you do:

  $ git rebase -i B~

In the editor that pops out, you choose `edit' for B, leave the others
unchanged.  Then git will checkout A for you, and wait for you to edit.
Now you can apply patch B in parts (by hand).

  $ git show B > patch
  $ # apply part1 of patch (assuming you are breaking it into 2 parts)
  $ git commit -a -m "Message for part1" # lets say this is B1
  $ # apply rest of the patch
  $ git commit -a -m "Message for the rest" # and this is B2
  $ git rebase --continue

Now your commit graph should be like this:

  A---B1---B2---C---D

Hope this helps,

-- 
Suvayu

Open source is the future. It sets us free.

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

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

Suvayu Ali <fatkasuvayu+linux@gmail.com> writes:

> On Mon, May 12, 2014 at 10:33:48AM +0200, Rainer M Krug wrote:
>> Eric Schulte <schulte.eric@gmail.com> writes:
>> 
>> > Rainer M Krug <Rainer@krugs.de> writes:
>> >
>> >> Bastien <bzg@gnu.org> writes:
>> >>
>> >>> Hi Rainer,
>> >>>
>> >>> Rainer M Krug <r.m.krug@gmail.com> writes:
>> >>>
>> >>>> I'll look at it again tomorrow and let you know as I made some changes
>> >>>> since then. Do you prefer one patch to several?
>> >>>
>> >>> Up to Eric's taste -- but in general I think a series of patches
>> >>> is better, it allows you to isolate and fix conflicts more easily.
>> >>
>> >
>> > I agree, multiple patches make future maintenance easier.
>> >
>> 
>> OK - I'll do so.
>> 
>> A little bit off-topic, is there a "git way" of splitting one patch into
>> several patches, if it was a single commit?
>
> Do an interactive rebase, and amend.
>
> Say this is the commit graph:
>
>   A---B---C---D
>
> You want to split B.  Then you do:
>
>   $ git rebase -i B~
>
> In the editor that pops out, you choose `edit' for B, leave the others
> unchanged.  Then git will checkout A for you, and wait for you to edit.
> Now you can apply patch B in parts (by hand).
>
>   $ git show B > patch
>   $ # apply part1 of patch (assuming you are breaking it into 2 parts)
>   $ git commit -a -m "Message for part1" # lets say this is B1
>   $ # apply rest of the patch
>   $ git commit -a -m "Message for the rest" # and this is B2
>   $ git rebase --continue
>
> Now your commit graph should be like this:
>
>   A---B1---B2---C---D
>
> Hope this helps,

Definitely. Sounds perfect. I will look at it a little bit later and
come back if I have any problems.

Thanks,

Rainer


-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Queestion concerning lists - was: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

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

Rainer M Krug <Rainer@krugs.de> writes:

> Eric Schulte <schulte.eric@gmail.com> writes:
>
>> Rainer M Krug <Rainer@krugs.de> writes:
>>
>>> Bastien <bzg@gnu.org> writes:
>>>
>>>> Hi Rainer,
>>>>
>>>> Rainer M Krug <r.m.krug@gmail.com> writes:
>>>>
>>>>> I'll look at it again tomorrow and let you know as I made some changes
>>>>> since then. Do you prefer one patch to several?
>>>>
>>>> Up to Eric's taste -- but in general I think a series of patches
>>>> is better, it allows you to isolate and fix conflicts more easily.
>>>
>>
>> I agree, multiple patches make future maintenance easier.
>>
>
> OK - I'll do so.
>
> A little bit off-topic, is there a "git way" of splitting one patch into
> several patches, if it was a single commit?
>
>>>> I missed some previous discussion in this thread.  Are these patches
>>>> ready to be applied as is?
>>>>
>>>
>>>
>>> IMO, the patches hard coded behaviors that would better be customizable
>>> and optional. 
>>>
>>> Rainer and I had some back and forth about this -- see the thread.
>>
>> With respect to these points, I'm inclined to agree with Charles in the
>> following.
>
> OK - see further comments below.
>
>>
>>> All you have to do is add this:
>>>
>>> (defvar org-babel-R-assign-elisp-function 'org-babel-R-assign-elisp
>>>   "Name or definition of function to handle `:var name=value'
>>> header args."
>>>   )
>>>
>>> and change one line in org-babel-variable-assignments:R from
>>>
>>>     (org-babel-R-assign-elisp to
>>>
>>>    (funcall org-babel-R-assign-elisp-function
>>>
>>> and the user can provide her own elisp assignment function.
>>>
>>> This gives users who want special behavior like creating something
>>> other than a data.frame the option of providing their own function.
>>
>> Would such a customization variable be difficult to add to your patches?
>
> I don't think so - I'll look into it.
>
>> If not would you mind submitting a version of the patches split into
>> multiple commits with as much of the hard-coded R code as feasible
>> placed into customizable variables along the lines of the
>> `org-babel-R-assign-elisp-function' variable suggested by Charles.  
>
> I am thinking of actually not providing the R code in org-variables, but
> to put them into R files and to source them. By doing this, the
> customization could be done in R, which will be much easier for R users
> then to customize emacs variables.
>
> These would be sourced and stored into an environment "org:functions",
> using the same approach as ESS is using to store functions into an
> environment "ESSR". I would then put the variables transfered into
> "org:variables". These environments would only exist in the search path,
> and not overwrite any user set objects in R.

I am working on it, but I have a question concerning strings and lists
in elisp.

In the function  org-babel-execute:R it says:

,----
| (inside
| 		   (list (org-babel-expand-body:R body params graphics-file)))
`----

I now want to convert "inside" to a comma separated string. I am doing
now the following:

,----
| (replace-regexp-in-string "\n" ", " (format "%s" inside))
`----

but this does not look elegant to me, as I am converting inside to a
string and then do a replace. There is the function mapconcat, but I
don't get it to work:

,----
| (replace-regexp-in-string "\n" ", " (format "%s" inside))
| "(     createOrgVariablesEnvironment(), plot(1))"
| 
| (mapconcat 'identity inside ", ")
| "     createOrgVariablesEnvironment()
| plot(1)"
`----

What am I missing?

Thanks,

Rainer

>
> As it needs to be sourced for each R process once, the right place would
> be in  org-babel-R-initiate-session - correct?
>
> What would be the best place to put these R files? 
>
>> One lesson I've certainly learned from the Org-mode mailing list is
>> that you can't anticipate all of the ways that your code will be used,
>> so up-front customizability generally pays off.
>
> OK - point taken - and I am definitely one of those users who thinks
> about unusual usages of certain features.
>
> Cheers,
>
> Rainer
>
>>
>> Thanks,
>> Eric
>>
>>>
>>> Thanks
>>>
>>> Rainer

-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Re: [babel][PATCHES] ob-R patches for review

[Eric Schulte]

>> If not would you mind submitting a version of the patches split into
>> multiple commits with as much of the hard-coded R code as feasible
>> placed into customizable variables along the lines of the
>> `org-babel-R-assign-elisp-function' variable suggested by Charles.  
>
> I am thinking of actually not providing the R code in org-variables, but
> to put them into R files and to source them. By doing this, the
> customization could be done in R, which will be much easier for R users
> then to customize emacs variables.
>

I think this is a bad idea, such external R source files may be hard to
manage and load reliably across systems and it is not clear where they
should live in the Org-mode repository.  Additionally, if the variables
simply hold R code text, then users can easily initialize them from R
files locally with something like the following.

    (setq org-babel-R-assign-elisp-function
          (with-temp-buffer
            (insert-file-contents-literally "personal.R")
            (buffer-string)))

I think this approach is much simpler.

Best,
Eric

>
> These would be sourced and stored into an environment "org:functions",
> using the same approach as ESS is using to store functions into an
> environment "ESSR". I would then put the variables transfered into
> "org:variables". These environments would only exist in the search path,
> and not overwrite any user set objects in R.
>
> As it needs to be sourced for each R process once, the right place would
> be in  org-babel-R-initiate-session - correct?
>
> What would be the best place to put these R files? 
>
>> One lesson I've certainly learned from the Org-mode mailing list is
>> that you can't anticipate all of the ways that your code will be used,
>> so up-front customizability generally pays off.
>
> OK - point taken - and I am definitely one of those users who thinks
> about unusual usages of certain features.
>
> Cheers,
>
> Rainer
>
>>
>> Thanks,
>> Eric
>>
>>>
>>> Thanks
>>>
>>> Rainer

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

Re: Queestion concerning lists - was: [babel][PATCHES] ob-R patches for review

[Eric Schulte]

>
> I am working on it, but I have a question concerning strings and lists
> in elisp.
>
> In the function  org-babel-execute:R it says:
>
> ,----
> | (inside
> | 		   (list (org-babel-expand-body:R body params graphics-file)))
> `----
>
> I now want to convert "inside" to a comma separated string. I am doing
> now the following:
>
> ,----
> | (replace-regexp-in-string "\n" ", " (format "%s" inside))
> `----
>
> but this does not look elegant to me, as I am converting inside to a
> string and then do a replace. There is the function mapconcat, but I
> don't get it to work:
>
> ,----
> | (replace-regexp-in-string "\n" ", " (format "%s" inside))
> | "(     createOrgVariablesEnvironment(), plot(1))"
> | 
> | (mapconcat 'identity inside ", ")
> | "     createOrgVariablesEnvironment()
> | plot(1)"
> `----
>
> What am I missing?
>

The following should work.

  (mapconcat #'identity inside ", ")

If that doesn't give the expected result, maybe share an example value
of inside, with the expected results.

Best,
Eric

>
> Thanks,
>
> Rainer
>
>>
>> As it needs to be sourced for each R process once, the right place would
>> be in  org-babel-R-initiate-session - correct?
>>
>> What would be the best place to put these R files? 
>>
>>> One lesson I've certainly learned from the Org-mode mailing list is
>>> that you can't anticipate all of the ways that your code will be used,
>>> so up-front customizability generally pays off.
>>
>> OK - point taken - and I am definitely one of those users who thinks
>> about unusual usages of certain features.
>>
>> Cheers,
>>
>> Rainer
>>
>>>
>>> Thanks,
>>> Eric
>>>
>>>>
>>>> Thanks
>>>>
>>>> Rainer

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

Re: [babel][PATCHES] ob-R patches for review

[Rainer M Krug]

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

Eric Schulte <schulte.eric@gmail.com> writes:

>>> If not would you mind submitting a version of the patches split into
>>> multiple commits with as much of the hard-coded R code as feasible
>>> placed into customizable variables along the lines of the
>>> `org-babel-R-assign-elisp-function' variable suggested by Charles.  
>>
>> I am thinking of actually not providing the R code in org-variables, but
>> to put them into R files and to source them. By doing this, the
>> customization could be done in R, which will be much easier for R users
>> then to customize emacs variables.
>>
>
> I think this is a bad idea, such external R source files may be hard to
> manage and load reliably across systems 

I see your points, but I am looking at ESS (which is loading .R files as
well into the ESSR environment) and whose loading mechanism I want
to piggy back the loading of .R for org. If I understand the variable
transfer from org to R correctly, it anyway only works on local R
sessions, which makes it even easier to do then ESS which also caters
for remote R sessions.
My idea is to have all R code in one directory and to let ESS load it
upon initialization of ESS (which is a dependency of running R from org
anyway, if I am not mistaken). I have a prototype working, and will keep
you posted. The complication would be that a newer version of ESS would
be needed.

The other option would be to just copy the code ESS uses into org, which
would make the process independent of changes in ESS. But I don't like
the duplication of code.

> and it is not clear where they should live in the Org-mode repository.

I would suggest in a etc/R. 

> Additionally, if the variables simply hold R code text, then users can
> easily initialize them from R files locally with something like the
> following.
>
>     (setq org-babel-R-assign-elisp-function
>           (with-temp-buffer
>             (insert-file-contents-literally "personal.R")
>             (buffer-string)))
>
> I think this approach is much simpler.

True - but I like the simplicity of being able to customize the
behavior of org-babel-R by writing an R function without having to thin
about elisp. But maybe there is a way of doing both...

Thanks for your comments,

Rainer

>
> Best,
> Eric
>
>>
>> These would be sourced and stored into an environment "org:functions",
>> using the same approach as ESS is using to store functions into an
>> environment "ESSR". I would then put the variables transfered into
>> "org:variables". These environments would only exist in the search path,
>> and not overwrite any user set objects in R.
>>
>> As it needs to be sourced for each R process once, the right place would
>> be in  org-babel-R-initiate-session - correct?
>>
>> What would be the best place to put these R files? 
>>
>>> One lesson I've certainly learned from the Org-mode mailing list is
>>> that you can't anticipate all of the ways that your code will be used,
>>> so up-front customizability generally pays off.
>>
>> OK - point taken - and I am definitely one of those users who thinks
>> about unusual usages of certain features.
>>
>> Cheers,
>>
>> Rainer
>>
>>>
>>> Thanks,
>>> Eric
>>>
>>>>
>>>> Thanks
>>>>
>>>> Rainer

-- 
Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany)

Centre of Excellence for Invasion Biology
Stellenbosch University
South Africa

Tel :       +33 - (0)9 53 10 27 44
Cell:       +33 - (0)6 85 62 59 98
Fax :       +33 - (0)9 58 10 27 44

Fax (D):    +49 - (0)3 21 21 25 22 44

email:      Rainer@krugs.de

Skype:      RMkrug

PGP: 0x0F52F982

[-- Attachment #2: Type: application/pgp-signature, Size: 494 bytes --]

Re: [babel][PATCHES] ob-R patches for review

[Charles C. Berry]

On Mon, 12 May 2014, Rainer M Krug wrote:

> Eric Schulte <schulte.eric@gmail.com> writes:
>
>>>> If not would you mind submitting a version of the patches split into
>>>> multiple commits with as much of the hard-coded R code as feasible
>>>> placed into customizable variables along the lines of the
>>>> `org-babel-R-assign-elisp-function' variable suggested by Charles.
>>>
>>> I am thinking of actually not providing the R code in org-variables, but
>>> to put them into R files and to source them. By doing this, the
>>> customization could be done in R, which will be much easier for R users
>>> then to customize emacs variables.
>>>
>>
>> I think this is a bad idea, such external R source files may be hard to
>> manage and load reliably across systems
>
> I see your points, but I am looking at ESS (which is loading .R files as
> well into the ESSR environment) and whose loading mechanism I want
> to piggy back the loading of .R for org. If I understand the variable
> transfer from org to R correctly, it anyway only works on local R
> sessions, which makes it even easier to do then ESS which also caters
> for remote R sessions.
> My idea is to have all R code in one directory and to let ESS load it
> upon initialization of ESS (which is a dependency of running R from org
> anyway, if I am not mistaken).

Not quite. You can run R src blocks without (require 'ess) if they require 
no session. (I do not claim that anyone actually runs R src blocks who 
lacks a working ESS installation, just saying...)


> I have a prototype working, and will keep
> you posted. The complication would be that a newer version of ESS would
> be needed.
>

Maybe load the ESS and R files if

   (and (fboundp 'ess-version)
        (not (string<
              (ess-version)
              "ess-version: <least-version>")))

and fallback to the existing version of ob-R.el (or something like it) 
otherwise.

> The other option would be to just copy the code ESS uses into org, which
> would make the process independent of changes in ESS. But I don't like
> the duplication of code.
>
>> and it is not clear where they should live in the Org-mode repository.
>
> I would suggest in a etc/R.
>

IIUC, someone on ESS core will support your effort. So maybe you can have 
an etc/ESSR/R/org-mode.R file.

Since you will have a mix of elisp and R, it might make sense to have 
minimal callouts on the org-mode side to an elisp wrapper on the ESS side. 
Then you can maintain the trickier elisp on the ESS side, so changes in 
elisp that require changes in R or vice versa can be made in unison.

>> Additionally, if the variables simply hold R code text, then users can
>> easily initialize them from R files locally with something like the
>> following.
>>
>>     (setq org-babel-R-assign-elisp-function
>>           (with-temp-buffer
>>             (insert-file-contents-literally "personal.R")
>>             (buffer-string)))
>>
>> I think this approach is much simpler.
>
> True - but I like the simplicity of being able to customize the
> behavior of org-babel-R by writing an R function without having to thin
> about elisp. But maybe there is a way of doing both...
>

noweb will do it. Quote the chunk like this:

#+BEGIN_SRC emacs-lisp :noweb yes :tangle elisp-R.el
   (setq rlines "
<<r-code>>")
#+END_SRC


HTH,

Chuck


Charles C. Berry                            Dept of Family/Preventive Medicine
cberry at ucsd edu			    UC San Diego
http://famprevmed.ucsd.edu/faculty/cberry/  La Jolla, CA 92093-0901

Re: [babel][PATCHES] ob-R patches for review

[Charles C. Berry]

On Fri, 16 May 2014, Rainer M Krug wrote:

> Sorry for coming back to your suggestions so late, but I did some
> thinking about the approach. Comments inline below.

No problem. I am about to leave town and my email for a couple of weeks,
so I might not be able to reply to further correspondence till June.

Comments inline below.

Chuck
>
> "Charles C. Berry" <ccberry@ucsd.edu> writes:
>
>> On Mon, 12 May 2014, Rainer M Krug wrote:
>>
>>> Eric Schulte <schulte.eric@gmail.com> writes:
>>>

[...]

[Rainer writes]
>>> My idea is to have all R code in one directory and to let ESS load it
>>> upon initialization of ESS (which is a dependency of running R from org
>>> anyway, if I am not mistaken).
>>
[Chuck]
>> Not quite. You can run R src blocks without (require 'ess) if they require
>> no session. (I do not claim that anyone actually runs R src blocks who
>> lacks a working ESS installation, just saying...)
>
[Rainer again]
> Hm - would it then be possible to add ESS as a requirement for running R
> code, as it is already mentioned on [1] as a requirement?

I'd be surprised if it would have any impact on users. Who uses R and
org-mode without ESS? But as you say it is listed as a requirement.

[...]

>
> I am now leaning towards the following approach:
>
> New behavior (environment):
> 1) Define the R code in emacs variables, i.e. customizable in emacs.
> 2) Define a function to load the code into an R environment - not customizable.
>
> ## upon creation of the R session (i.e. only once for each R session)

Maybe wait till the first variable transfer is called, if it ever is.

>
> 3) use the load function to load the code from the emacs variables into
> an R environment called "org:functions"
> 4) load all .R files in the directory ~/.orgFunctions/ into
> "org:functions". This will mean that when functions of the same name as
> loaded earlier will be overwritten, i.e. the functions are customizable
> by R code only.
>
> ## before variable transfer at each execution of a code block

Only if there is a variable transfer, of course.

>
> 5) source all .R files in the directory RWorkingDirectory/.orgFunctions/
> i.e. they are not loaded into a specific environment. Reasoning: further
> customization possible, kind of an "R package light" approach.
>
> 6) use the load functions (as defied initially in (4), (3) or in () from
> to load the variables. If they are loaded into an environment or not
> depends on the definition of the functions.
>
> Concerning old versus new behavior, I would actually suggest to put this
> into the R functions: one could add an argument "variablesInEnvironment"
> which would, if set, be the name of the environment, i.e. default value
> "org:variables", if NULL it would load into .GlobalEnv, i.e. the old
> behaviour. In org, one could have a variable
> org-babel-R-variable-transfer-type (or whatever the proper name should
> be) and to make it user configurable:
>
> - null :: old behaviour
> - "org:variables" (default) :: variables into environment called
> "org:variables"
> - other string :: name of the environment to be used
>
> As stated earlier, as I think that name clashes are more unlikely with
> the new approach, I would suggest to make "org:variables" the default.
>

The above sounds good.

If you do not wish to implement the `org-babel-R-assign-elisp-function' 
variable approach I suggested, I'll make it a TINYCHANGE when I return in 
June. It will not impact on what you propose. However, if your 
implementation makes it possible for arbitrary elisp variables to be 
handled in R - I might not bother.

[...]

>>>
>>> True - but I like the simplicity of being able to customize the
>>> behavior of org-babel-R by writing an R function without having to thin
>>> about elisp. But maybe there is a way of doing both...
>>>
>>
>> noweb will do it. Quote the chunk like this:
>>
>> #+BEGIN_SRC emacs-lisp :noweb yes :tangle elisp-R.el
>>    (setq rlines "
>> <<r-code>>")
>> #+END_SRC
>
> I must admit, I have never used macros and noweb expansion...
>
> If you could give me your opinions on this approach, I would look into
> implementing it.

It is just a labor saving suggestion for you -- it would not show up
in the final elisp. Put all your elisp and R code in src blocks in a
*.org file. You can edit and format the R code in R src blocks and
likewise the elisp. A src block like the one above will import the R
into elisp as a string. You only tangle the elisp. So the benefit is
you have ease of editting R and writing unit tests for it for R code
that appears in elisp.

IMO, noweb is a good way to mix languages in org-mode. I use the R 
`inline' package to write C functions that R can call. R needs the code as 
strings. I write the C code in C src blocks then assign it in as an R 
character string in an R src block. I can write an R wrapper and unit test 
for each little snippet of C code, then when it all looks good join up the 
snippets by putting multiple noweb references on successive lines.


>
>
> Footnotes:
> [1]  http://orgmode.org/worg/org-contrib/babel/languages.html
>
[...]

Re: [babel][PATCHES] ob-R patches for review

[Eric Schulte]

>> Additionally, if the variables simply hold R code text, then users can
>> easily initialize them from R files locally with something like the
>> following.
>>
>>     (setq org-babel-R-assign-elisp-function
>>           (with-temp-buffer
>>             (insert-file-contents-literally "personal.R")
>>             (buffer-string)))
>>
>> I think this approach is much simpler.
>
> True - but I like the simplicity of being able to customize the
> behavior of org-babel-R by writing an R function without having to thin
> about elisp. But maybe there is a way of doing both...
>

I guess one man's simplicity is another confusing magic.  I don't see
the difficulty in having to change the value of an elisp variable, to
change the R code executed by an Emacs process, in fact I think this is
the most straight forward and expected way to do such customization.

> New behavior (environment):
> 1) Define the R code in emacs variables, i.e. customizable in emacs.
> 2) Define a function to load the code into an R environment - not customizable.

The above sounds good to me.  It might be nice to also include a
customizable variable which controls whether the function in (2) is run
at all.

Best,

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D (see https://u.fsf.org/yw)