[PATCH] Change default ob-python session command to match run-python

[PATCH] Change default ob-python session command to match run-python

[Jack Kamm]

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

The attached patch changes the default behavior of ob-python sessions,
to respect python-shell-interpreter(-args) when starting an interactive
session.

It also allows separate customization of the default Python command for
nonsessions and sessions.

This mainly benefits IPython users.  IPython isn't suitable as the
default command for nonsessions, but until now there isn't a way to set
it as the default interpreter for sessions only.

Another benefit is to promote greater consistency between ob-python.el
and upstream python.el, in particular for shells started with
run-python.  If a user configures python-shell-interpreter(-args), then
ob-python will respect those settings now.

As explained in the NEWS entry, this change should have no effect on
users who previously configured `org-babel-python-command', or on users
who stick to the default `python-shell-interpreter'.  But I submit the
patch for review before applying it, because it involves changing the
default values of some custom variables.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-python-Changed-options-for-default-Python-command.patch --]
[-- Type: text/x-patch, Size: 7417 bytes --]

From a49ddcb6ef72cfefab400e36e6d4a19e869c47a1 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Fri, 29 Dec 2023 13:22:18 -0800
Subject: [PATCH] ob-python: Changed options for default Python command

ob-python will now use the same settings as `run-python' when starting
interactive sessions, by default.

* lisp/ob-python.el (org-babel-python-command): Changed to have
default value of nil.
(org-babel-python-command-session): New option to control default
session Python command.
(org-babel-python-command-nonsession): New option to control default
nonsession Python command.
(org-babel-execute:python): Set `org-babel-python-command-session' and
`org-babel-python-command-nonsession' using :python header arg.
(org-babel-python-initiate-session-by-key): Call `run-python' without
CMD arg.  Instead, set `python-shell-interpreter' and
`python-shell-interpreter-args' when needed.
(org-babel-python-evaluate-external-process): Use
`org-babel-python-command-nonsession' to start nonsession Python.
---
 etc/ORG-NEWS      | 30 ++++++++++++++++++++++++
 lisp/ob-python.el | 58 ++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 75 insertions(+), 13 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index c54473f55..688735a5b 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -551,6 +551,36 @@ Currently implemented options are:
 The capture template expansion element =%K= creates links using
 ~org-store-link~, which respects the values of ~org-id-link-to-use-id~.
 
+*** Changes to ~org-babel-python-command~, and new session/nonsession specific options
+
+The default Python command used by interactive sessions has been
+changed to match ~python-shell-interpreter~ and
+~python-shell-interpreter-args~ by default.  The default Python
+command for nonsessions has not changed.
+
+New options ~org-babel-python-command-nonsession~ and
+~org-babel-python-command-session~ control the default Python command
+for nonsessions and sessions, respectively.  By default,
+~org-babel-python-command-session~ is nil, which means to use the
+configuration for ~python-shell-interpreter(-args)~ as default.
+
+The old option ~org-babel-python-command~ has been changed to have
+default value of nil.  When non-nil, it overrides both
+~org-babel-python-command-nonsession~ and
+~org-babel-python-command-session~.  Therefore, users who had
+previously set ~org-babel-python-command~ will not experience any
+changes.
+
+Likewise, users who had neither set ~org-babel-python-command~ nor
+~python-shell-interpreter(-args)~ will not see any changes -- ~python~
+remains the default command.
+
+The main change will be for users who did not configure
+~org-babel-python-command~, but did configure
+~python-shell-interpreter~, e.g. to use IPython.  In this case,
+~ob-python~ will now start interactive sessions in a more consistent
+manner with ~run-python~.
+
 ** New features
 *** =ob-plantuml.el=: Support tikz file format output
 
diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 6b3a608c8..2b17f41fe 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -46,10 +46,31 @@ (defconst org-babel-header-args:python
     (python . :any))
   "Python-specific header arguments.")
 
-(defcustom org-babel-python-command "python"
-  "Name of the command for executing Python code."
-  :version "24.4"
-  :package-version '(Org . "8.0")
+(defcustom org-babel-python-command nil
+  "Name of the command for interactive and non-interactive Python code.
+If set, it overrides `org-babel-python-command-session' and
+`org-babel-python-command-nonsession'."
+  :version "29.2"
+  :package-version '(Org . "9.7")
+  :group 'org-babel
+  :type '(choice string (const nil)))
+
+(defcustom org-babel-python-command-session nil
+  "Name of the command for starting interactive Python sessions.
+If `nil' (the default), uses the values from
+`python-shell-interpreter' and `python-shell-interpreter-args'.
+If `org-babel-python-command' is set, then it overrides this
+option."
+  :version "29.2"
+  :package-version '(Org . "9.7")
+  :group 'org-babel
+  :type '(choice string (const nil)))
+
+(defcustom org-babel-python-command-nonsession "python"
+  "Name of the command for executing non-interactive Python code.
+If `org-babel-python-command' is set, then it overrides this option."
+  :version "29.2"
+  :package-version '(Org . "9.7")
   :group 'org-babel
   :type 'string)
 
@@ -70,9 +91,14 @@ (defcustom org-babel-python-None-to 'hline
 (defun org-babel-execute:python (body params)
   "Execute Python BODY according to PARAMS.
 This function is called by `org-babel-execute-src-block'."
-  (let* ((org-babel-python-command
+  (let* ((org-babel-python-command-nonsession
+	  (or (cdr (assq :python params))
+	      org-babel-python-command
+              org-babel-python-command-nonsession))
+         (org-babel-python-command-session
 	  (or (cdr (assq :python params))
-	      org-babel-python-command))
+	      org-babel-python-command
+              org-babel-python-command-session))
 	 (session (org-babel-python-initiate-session
 		   (cdr (assq :session params))))
 	 (graphics-file (and (member "graphics" (assq :result-params params))
@@ -267,13 +293,20 @@ (defun org-babel-python-initiate-session-by-key (&optional session)
     (let* ((session (if session (intern session) :default))
            (py-buffer (or (org-babel-python-session-buffer session)
                           (org-babel-python-with-earmuffs session)))
-	   (cmd (if (member system-type '(cygwin windows-nt ms-dos))
-		    (concat org-babel-python-command " -i")
-		  org-babel-python-command))
            (python-shell-buffer-name
 	    (org-babel-python-without-earmuffs py-buffer))
            (existing-session-p (comint-check-proc py-buffer)))
-      (run-python cmd)
+      (if org-babel-python-command-session
+          (let* ((cmd-split (split-string-and-unquote
+                             org-babel-python-command-session))
+                 (python-shell-interpreter (car cmd-split))
+                 (python-shell-interpreter-args
+                  (append (cdr cmd-split)
+                          (when (member system-type
+                                        '(cygwin windows-nt ms-dos))
+                            (list "-i")))))
+            (run-python))
+        (run-python))
       (with-current-buffer py-buffer
         (if existing-session-p
             ;; Session was created outside Org.  Assume first prompt
@@ -374,7 +407,7 @@ (defun org-babel-python-evaluate-external-process
 non-nil, then save graphical results to that file instead."
   (let ((raw
          (pcase result-type
-           (`output (org-babel-eval org-babel-python-command
+           (`output (org-babel-eval org-babel-python-command-nonsession
 				    (concat preamble (and preamble "\n")
                                             (if graphics-file
                                                 (format org-babel-python--output-graphics-wrapper
@@ -382,8 +415,7 @@ (defun org-babel-python-evaluate-external-process
                                               body))))
            (`value (let ((results-file (or graphics-file
 				           (org-babel-temp-file "python-"))))
-		     (org-babel-eval
-		      org-babel-python-command
+		     (org-babel-eval org-babel-python-command-nonsession
 		      (concat
 		       preamble (and preamble "\n")
 		       (format
-- 
2.43.0

Re: [PATCH] Change default ob-python session command to match run-python

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

> The attached patch changes the default behavior of ob-python sessions,
> to respect python-shell-interpreter(-args) when starting an interactive
> session.

Thanks!
I have several comments.

> -(defcustom org-babel-python-command "python"
> -  "Name of the command for executing Python code."
> -  :version "24.4"
> -  :package-version '(Org . "8.0")
> +(defcustom org-babel-python-command nil
> +  "Name of the command for interactive and non-interactive Python code.
> +If set, it overrides `org-babel-python-command-session' and
> +`org-babel-python-command-nonsession'."

What about default value being 'auto? That would be more explicit.

> +  :version "29.2"

:version tags should not be used when we use :package-version - we drop
them as we change the defcustom that still has them.
See 15.1 Common Item Keywords section of Elisp manual:

    ‘:package-version '(PACKAGE . VERSION)’
         This keyword specifies that the item was first introduced in
         PACKAGE version VERSION, or that its meaning or default value was
         changed in that version.  This keyword takes priority over
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
         ‘:version’
         ^^^^^^^^^^

> +(defcustom org-babel-python-command-session nil
> +  "Name of the command for starting interactive Python sessions.
> +If `nil' (the default), uses the values from
> +`python-shell-interpreter' and `python-shell-interpreter-args'.
> +If `org-babel-python-command' is set, then it overrides this
> +option."
> ...
> +(defcustom org-babel-python-command-nonsession "python"
> +  "Name of the command for executing non-interactive Python code.
> +If `org-babel-python-command' is set, then it overrides this option."

Unless I miss something, it looks like you also allow command arguments,
not just command name as the value. You may need to adjust the docstring
to reflect this fact.

>  (defun org-babel-execute:python (body params)
>    "Execute Python BODY according to PARAMS.
>  This function is called by `org-babel-execute-src-block'."
> -  (let* ((org-babel-python-command
> +  (let* ((org-babel-python-command-nonsession
> +	  (or (cdr (assq :python params))
> +	      org-babel-python-command
> +              org-babel-python-command-nonsession))
> +         (org-babel-python-command-session
>  	  (or (cdr (assq :python params))
> -	      org-babel-python-command))
> +	      org-babel-python-command
> +              org-babel-python-command-session))

> @@ -374,7 +407,7 @@ (defun org-babel-python-evaluate-external-process
>  non-nil, then save graphical results to that file instead."
>    (let ((raw
>           (pcase result-type
> -           (`output (org-babel-eval org-babel-python-command
> +           (`output (org-babel-eval org-babel-python-command-nonsession
>

I am slightly concerned about
`org-babel-python-evaluate-external-process' relying upon being called
from `org-babel-execute:python' that let-binds
`org-babel-python-nonsession'.

Since `org-babel-python-evaluate-external-process' is a public function,
it may also be called independently. And it will not obey
`org-babel-python-command' then.

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

Re: [PATCH] Change default ob-python session command to match run-python

[Jack Kamm]

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

Ihor Radchenko <yantar92@posteo.net> writes:

> Thanks!
> I have several comments.

Thanks for the feedback! I attach a followup patch in response. If all
looks good, I will squash before applying.

>> -(defcustom org-babel-python-command "python"
>> -  "Name of the command for executing Python code."
>> -  :version "24.4"
>> -  :package-version '(Org . "8.0")
>> +(defcustom org-babel-python-command nil
>> +  "Name of the command for interactive and non-interactive Python code.
>> +If set, it overrides `org-babel-python-command-session' and
>> +`org-babel-python-command-nonsession'."
>
> What about default value being 'auto? That would be more explicit.

Sure, I have updated `org-babel-python-command(-session)' to use auto as
default.

> :version tags should not be used when we use :package-version - we drop
> them as we change the defcustom that still has them.

I've dropped the :version tags for the updated variables. I can also
make a separate commit later to drop :version from other ob-python
variables like `org-babel-python-hline-to' and `org-babel-python-None-to'.

> Unless I miss something, it looks like you also allow command arguments,
> not just command name as the value. You may need to adjust the docstring
> to reflect this fact.

I've updated the docstrings to say they are for the command including
arguments.

>>  (defun org-babel-execute:python (body params)
>>    "Execute Python BODY according to PARAMS.
>>  This function is called by `org-babel-execute-src-block'."
>> -  (let* ((org-babel-python-command
>> +  (let* ((org-babel-python-command-nonsession
>> +	  (or (cdr (assq :python params))
>> +	      org-babel-python-command
>> +              org-babel-python-command-nonsession))
>> +         (org-babel-python-command-session
>>  	  (or (cdr (assq :python params))
>> -	      org-babel-python-command))
>> +	      org-babel-python-command
>> +              org-babel-python-command-session))
>
>> @@ -374,7 +407,7 @@ (defun org-babel-python-evaluate-external-process
>>  non-nil, then save graphical results to that file instead."
>>    (let ((raw
>>           (pcase result-type
>> -           (`output (org-babel-eval org-babel-python-command
>> +           (`output (org-babel-eval org-babel-python-command-nonsession
>>
>
> I am slightly concerned about
> `org-babel-python-evaluate-external-process' relying upon being called
> from `org-babel-execute:python' that let-binds
> `org-babel-python-nonsession'.
>
> Since `org-babel-python-evaluate-external-process' is a public function,
> it may also be called independently. And it will not obey
> `org-babel-python-command' then.

That's a good catch.

I've fixed this by reverting `org-babel-execute:python' to set
`org-babel-python-command' (not `org-babel-python-command-nonsession').
Then, `org-babel-python-evaluate-external-process' checks both variables
to decide what the command is.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Incorporate-feedback-from-Ihor-to-be-squashed-before.patch --]
[-- Type: text/x-patch, Size: 6236 bytes --]

From 91352d03c897dc546f57f48a14847cc78e74ec85 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sat, 30 Dec 2023 20:44:46 -0800
Subject: [PATCH 2/2] Incorporate feedback from Ihor; to be squashed before
 applying

---
 lisp/ob-python.el | 67 ++++++++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 30 deletions(-)

diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 2b17f41fe..5251c3b33 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -46,30 +46,27 @@ (defconst org-babel-header-args:python
     (python . :any))
   "Python-specific header arguments.")
 
-(defcustom org-babel-python-command nil
-  "Name of the command for interactive and non-interactive Python code.
-If set, it overrides `org-babel-python-command-session' and
-`org-babel-python-command-nonsession'."
-  :version "29.2"
+(defcustom org-babel-python-command 'auto
+  "Command (including arguments) for interactive and non-interactive Python code.
+When not `auto', it overrides `org-babel-python-command-session'
+and `org-babel-python-command-nonsession'."
   :package-version '(Org . "9.7")
   :group 'org-babel
-  :type '(choice string (const nil)))
+  :type '(choice string (const auto)))
 
-(defcustom org-babel-python-command-session nil
-  "Name of the command for starting interactive Python sessions.
-If `nil' (the default), uses the values from
+(defcustom org-babel-python-command-session 'auto
+  "Command (including arguments) for starting interactive Python sessions.
+If `auto' (the default), uses the values from
 `python-shell-interpreter' and `python-shell-interpreter-args'.
 If `org-babel-python-command' is set, then it overrides this
 option."
-  :version "29.2"
   :package-version '(Org . "9.7")
   :group 'org-babel
-  :type '(choice string (const nil)))
+  :type '(choice string (const auto)))
 
 (defcustom org-babel-python-command-nonsession "python"
-  "Name of the command for executing non-interactive Python code.
+  "Command (including arguments) for executing non-interactive Python code.
 If `org-babel-python-command' is set, then it overrides this option."
-  :version "29.2"
   :package-version '(Org . "9.7")
   :group 'org-babel
   :type 'string)
@@ -91,14 +88,9 @@ (defcustom org-babel-python-None-to 'hline
 (defun org-babel-execute:python (body params)
   "Execute Python BODY according to PARAMS.
 This function is called by `org-babel-execute-src-block'."
-  (let* ((org-babel-python-command-nonsession
+  (let* ((org-babel-python-command
 	  (or (cdr (assq :python params))
-	      org-babel-python-command
-              org-babel-python-command-nonsession))
-         (org-babel-python-command-session
-	  (or (cdr (assq :python params))
-	      org-babel-python-command
-              org-babel-python-command-session))
+	      org-babel-python-command))
 	 (session (org-babel-python-initiate-session
 		   (cdr (assq :session params))))
 	 (graphics-file (and (member "graphics" (assq :result-params params))
@@ -272,6 +264,20 @@ (defun org-babel-python--python-util-comint-end-of-output-p ()
      (buffer-substring-no-properties
       (car prompt) (cdr prompt)))))
 
+(defun org-babel-python--command (is-session)
+  "Helper function to return the Python command.
+This checks `org-babel-python-command', and then
+`org-babel-python-command-session' (if IS-SESSION) or
+`org-babel-python-command-nonsession' (if not IS-SESSION).  If
+IS-SESSION, this might return `nil', which means to use
+`python-shell-calculate-command'."
+  (or (unless (eq org-babel-python-command 'auto)
+        org-babel-python-command)
+      (if is-session
+          (unless (eq org-babel-python-command-session 'auto)
+            org-babel-python-command-session)
+        org-babel-python-command-nonsession)))
+
 (defvar-local org-babel-python--initialized nil
   "Flag used to mark that python session has been initialized.")
 (defun org-babel-python--setup-session ()
@@ -295,16 +301,17 @@ (defun org-babel-python-initiate-session-by-key (&optional session)
                           (org-babel-python-with-earmuffs session)))
            (python-shell-buffer-name
 	    (org-babel-python-without-earmuffs py-buffer))
-           (existing-session-p (comint-check-proc py-buffer)))
-      (if org-babel-python-command-session
-          (let* ((cmd-split (split-string-and-unquote
-                             org-babel-python-command-session))
+           (existing-session-p (comint-check-proc py-buffer))
+           (cmd (org-babel-python--command t)))
+      (if cmd
+          (let* ((cmd-split (split-string-and-unquote cmd))
                  (python-shell-interpreter (car cmd-split))
                  (python-shell-interpreter-args
-                  (append (cdr cmd-split)
-                          (when (member system-type
-                                        '(cygwin windows-nt ms-dos))
-                            (list "-i")))))
+                  (combine-and-quote-strings
+                   (append (cdr cmd-split)
+                           (when (member system-type
+                                         '(cygwin windows-nt ms-dos))
+                             (list "-i"))))))
             (run-python))
         (run-python))
       (with-current-buffer py-buffer
@@ -407,7 +414,7 @@ (defun org-babel-python-evaluate-external-process
 non-nil, then save graphical results to that file instead."
   (let ((raw
          (pcase result-type
-           (`output (org-babel-eval org-babel-python-command-nonsession
+           (`output (org-babel-eval (org-babel-python--command nil)
 				    (concat preamble (and preamble "\n")
                                             (if graphics-file
                                                 (format org-babel-python--output-graphics-wrapper
@@ -415,7 +422,7 @@ (defun org-babel-python-evaluate-external-process
                                               body))))
            (`value (let ((results-file (or graphics-file
 				           (org-babel-temp-file "python-"))))
-		     (org-babel-eval org-babel-python-command-nonsession
+		     (org-babel-eval (org-babel-python--command nil)
 		      (concat
 		       preamble (and preamble "\n")
 		       (format
-- 
2.43.0

Re: [PATCH] Change default ob-python session command to match run-python

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

> Thanks for the feedback! I attach a followup patch in response. If all
> looks good, I will squash before applying.

LGTM.

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