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>