* [PATCH] (Tiny) Tweak Python session null return value
@ 2020-02-17 16:24 Jack Kamm
2020-02-17 17:42 ` Bastien
2020-02-17 17:52 ` John Kitchin
0 siblings, 2 replies; 9+ messages in thread
From: Jack Kamm @ 2020-02-17 16:24 UTC (permalink / raw
To: emacs-orgmode, Bastien
[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]
Hi,
Below is a very small patch to Python session blocks, to make them
return a blank result (empty string) instead of None when there is no
return value.
Normally I would push this myself, but since we are so close to 9.4, I
thought it prudent to mail a patch and let the maintainers handle it. It
would be nice to include in 9.4, but not a big deal if I've missed the
window.
Now for an explanation of the patch: 9.4 changes Python session blocks
to fix several bugs and improve robustness overall [0]. However, there
is a cost to these fixes, which is that the session blocks can only
return a result when it is a top-level expression on the last line of
the block. If the last line is not a top-level expression, the block
would previously print "None". However, after some testing, I think this
is a little counter-intuitive, and it would be better if it returned a
blank (empty) result instead of "None".
[0] https://lists.gnu.org/archive/html/emacs-orgmode/2020-01/msg00190.html
Best,
Jack
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-python-Session-returns-empty-string-if-no-return-.patch --]
[-- Type: text/x-patch, Size: 2040 bytes --]
From 0b44c3f1c7454e7948cd34eb02995924046b6976 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Mon, 17 Feb 2020 08:11:49 -0800
Subject: [PATCH] ob-python: Session returns empty string if no return value
* lisp/ob-python.el (org-babel-python--eval-ast): Change sessions to
return an empty string, instead of None, if there is no return value.
---
etc/ORG-NEWS | 12 +++++++-----
lisp/ob-python.el | 2 +-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index b6ee443e4..1b5429870 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -12,13 +12,15 @@ Please send Org bug reports to mailto:emacs-orgmode@gnu.org.
* Version 9.4 (not yet released)
** Incompatible changes
-*** Python session return values must be top-level expression statements
+*** Change how Python sessions handle return values
Python blocks with ~:session :results value~ header arguments now only
-return a value if the last line is a top-level expression statement,
-otherwise the result is None. Also, None will now show up under
-"#+RESULTS:", as it already did with ~:results value~ for non-session
-blocks.
+return a value if the last line is a top-level expression
+statement. If the last line is not a top-level expression statement,
+the block will return a blank (empty) result.
+
+Also, a return value of ~None~ will now show up under "#+RESULTS:", as
+it already did for non-session blocks.
*** In HTML export, change on how outline-container-* is set
diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index de718b04b..b9e36dba0 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -260,7 +260,7 @@ (defconst org-babel-python--eval-ast "\
__org_babel_python_final.value), '<string>', 'eval'))
else:
exec(compile(__org_babel_python_ast, '<string>', 'exec'))
- __org_babel_python_final = None
+ __org_babel_python_final = ''
except Exception:
from traceback import format_exc
__org_babel_python_final = format_exc()
--
2.25.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] (Tiny) Tweak Python session null return value
2020-02-17 16:24 [PATCH] (Tiny) Tweak Python session null return value Jack Kamm
@ 2020-02-17 17:42 ` Bastien
2020-02-17 19:07 ` Jack Kamm
2020-02-17 17:52 ` John Kitchin
1 sibling, 1 reply; 9+ messages in thread
From: Bastien @ 2020-02-17 17:42 UTC (permalink / raw
To: Jack Kamm; +Cc: emacs-orgmode
Hi Jack,
Jack Kamm <jackkamm@gmail.com> writes:
> Below is a very small patch to Python session blocks, to make them
> return a blank result (empty string) instead of None when there is no
> return value.
>
> Normally I would push this myself, but since we are so close to 9.4, I
> thought it prudent to mail a patch and let the maintainers handle it. It
> would be nice to include in 9.4, but not a big deal if I've missed the
> window.
Applied, thanks for the patch and the precaution not to commit it
directly.
> Now for an explanation of the patch: 9.4 changes Python session blocks
> to fix several bugs and improve robustness overall [0]. However, there
> is a cost to these fixes, which is that the session blocks can only
> return a result when it is a top-level expression on the last line of
> the block. If the last line is not a top-level expression, the block
> would previously print "None". However, after some testing, I think this
> is a little counter-intuitive, and it would be better if it returned a
> blank (empty) result instead of "None".
I've seen you update the NEWS entry, which is good: is there a way to
present the enhancements in the "* New features" section? If you feel
like it, please advertize the enhancements there too.
Thanks,
--
Bastien
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] (Tiny) Tweak Python session null return value
2020-02-17 17:42 ` Bastien
@ 2020-02-17 19:07 ` Jack Kamm
2020-02-17 22:53 ` Bastien
0 siblings, 1 reply; 9+ messages in thread
From: Jack Kamm @ 2020-02-17 19:07 UTC (permalink / raw
To: Bastien; +Cc: emacs-orgmode
Hi Bastien,
> I've seen you update the NEWS entry, which is good: is there a way to
> present the enhancements in the "* New features" section? If you feel
> like it, please advertize the enhancements there too.
Given John's feedback, I now think it's better to put off this change to
9.5, if at all. So instead of updating NEWS, I'd prefer to revert this
commit for now. Sorry for the noise! Looking forward to 9.4.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] (Tiny) Tweak Python session null return value
2020-02-17 19:07 ` Jack Kamm
@ 2020-02-17 22:53 ` Bastien
0 siblings, 0 replies; 9+ messages in thread
From: Bastien @ 2020-02-17 22:53 UTC (permalink / raw
To: Jack Kamm; +Cc: emacs-orgmode
Hi Jack,
Jack Kamm <jackkamm@gmail.com> writes:
>> I've seen you update the NEWS entry, which is good: is there a way to
>> present the enhancements in the "* New features" section? If you feel
>> like it, please advertize the enhancements there too.
>
> Given John's feedback, I now think it's better to put off this change to
> 9.5, if at all. So instead of updating NEWS, I'd prefer to revert this
> commit for now. Sorry for the noise! Looking forward to 9.4.
Sure, I just reverted the commit.
This discussion made me realize that the various Babel backends may
have different approaches wrt evaluating blocks, returning output and
results, interpreting empty output/result, etc. The mere notion of a
"session" may differ from one language to another.
I wish we can push for more consistency here--perhaps the first stop
would be (1) to pick major backends and (2) document their behaviors.
I'm sure this could help enhancing Org babel code too.
Thanks,
--
Bastien
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] (Tiny) Tweak Python session null return value
2020-02-17 16:24 [PATCH] (Tiny) Tweak Python session null return value Jack Kamm
2020-02-17 17:42 ` Bastien
@ 2020-02-17 17:52 ` John Kitchin
2020-02-17 19:05 ` Jack Kamm
1 sibling, 1 reply; 9+ messages in thread
From: John Kitchin @ 2020-02-17 17:52 UTC (permalink / raw
To: Jack Kamm; +Cc: Bastien, org-mode-email
[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]
I think None is correct. If you don't specify a return value in Python,
then a function returns None. I would expect that to happen in a Python
block too.
John
-----------------------------------
Professor John Kitchin
Doherty Hall A207F
Department of Chemical Engineering
Carnegie Mellon University
Pittsburgh, PA 15213
412-268-7803
@johnkitchin
http://kitchingroup.cheme.cmu.edu
On Mon, Feb 17, 2020 at 11:25 AM Jack Kamm <jackkamm@gmail.com> wrote:
> Hi,
>
> Below is a very small patch to Python session blocks, to make them
> return a blank result (empty string) instead of None when there is no
> return value.
>
> Normally I would push this myself, but since we are so close to 9.4, I
> thought it prudent to mail a patch and let the maintainers handle it. It
> would be nice to include in 9.4, but not a big deal if I've missed the
> window.
>
> Now for an explanation of the patch: 9.4 changes Python session blocks
> to fix several bugs and improve robustness overall [0]. However, there
> is a cost to these fixes, which is that the session blocks can only
> return a result when it is a top-level expression on the last line of
> the block. If the last line is not a top-level expression, the block
> would previously print "None". However, after some testing, I think this
> is a little counter-intuitive, and it would be better if it returned a
> blank (empty) result instead of "None".
>
> [0] https://lists.gnu.org/archive/html/emacs-orgmode/2020-01/msg00190.html
>
> Best,
> Jack
>
>
>
[-- Attachment #2: Type: text/html, Size: 2332 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] (Tiny) Tweak Python session null return value
2020-02-17 17:52 ` John Kitchin
@ 2020-02-17 19:05 ` Jack Kamm
2020-02-17 19:45 ` John Kitchin
0 siblings, 1 reply; 9+ messages in thread
From: Jack Kamm @ 2020-02-17 19:05 UTC (permalink / raw
To: John Kitchin; +Cc: Bastien, org-mode-email
Hi John,
John Kitchin <jkitchin@andrew.cmu.edu> writes:
> I think None is correct. If you don't specify a return value in Python,
> then a function returns None. I would expect that to happen in a Python
> block too.
Hmm, OK, thanks for your intuition, it's useful feedback.
Working this out loud, I was considering the following sort of block:
#+begin_src python :session :results value
if some_condition:
True
else:
False
#+end_src
#+RESULTS:
: None
Ideally, it would return True/False, but the current implementation
cannot grab that result unfortunately. (In its defense, it at least
doesn't crash like it did before).
I was thinking not printing anything at all under "#+RESULTS" would be
less surprising than printing "None". But both are admittedly surprising
at first.
I agree your preference of returning "None" is the more technically
consistent behavior though.
I don't want to rush a decision now, I think I need to gather more user
feedback after the 9.4 release. I'll ask Bastien to revert that commit
and put this off to 9.5, rather than writing an updated NEWS entry like
he asked.
Either way, we'll need to update Worg to clearly document what's going
on here.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] (Tiny) Tweak Python session null return value
2020-02-17 19:05 ` Jack Kamm
@ 2020-02-17 19:45 ` John Kitchin
2020-02-17 20:45 ` Jack Kamm
0 siblings, 1 reply; 9+ messages in thread
From: John Kitchin @ 2020-02-17 19:45 UTC (permalink / raw
To: Jack Kamm; +Cc: Bastien, org-mode-email
[-- Attachment #1: Type: text/plain, Size: 2507 bytes --]
I can see why you would want to see True/False there, but to get the value,
you need to specifically return what you want because AFAIK the body is
wrapped in a function that is evaluated to get the value, it is not simply
the last thing that gets evaluated. Your example clarified to me at least
why it would be tricky to figure it out, you can't rely on the last line,
for example. I don't know if there is some special Python variable that
contains that.
It is also a little strange to me to put return specifically in like this:
#+begin_src python :session :results value
if some_condition:
a = True
else:
a = False
return a
#+end_src
or:
#+BEGIN_SRC python :results value
a = 1
return True if a else False
#+END_SRC
#+RESULTS:
: True
because it is not valid syntax in a script (try it with :results output),
but this is how it has been for ob-python all along when what you want is
the value returned.
John
-----------------------------------
Professor John Kitchin
Doherty Hall A207F
Department of Chemical Engineering
Carnegie Mellon University
Pittsburgh, PA 15213
412-268-7803
@johnkitchin
http://kitchingroup.cheme.cmu.edu
On Mon, Feb 17, 2020 at 2:06 PM Jack Kamm <jackkamm@gmail.com> wrote:
> Hi John,
>
> John Kitchin <jkitchin@andrew.cmu.edu> writes:
>
> > I think None is correct. If you don't specify a return value in Python,
> > then a function returns None. I would expect that to happen in a Python
> > block too.
>
> Hmm, OK, thanks for your intuition, it's useful feedback.
>
> Working this out loud, I was considering the following sort of block:
>
> #+begin_src python :session :results value
> if some_condition:
> True
> else:
> False
> #+end_src
>
> #+RESULTS:
> : None
>
> Ideally, it would return True/False, but the current implementation
> cannot grab that result unfortunately. (In its defense, it at least
> doesn't crash like it did before).
>
> I was thinking not printing anything at all under "#+RESULTS" would be
> less surprising than printing "None". But both are admittedly surprising
> at first.
>
> I agree your preference of returning "None" is the more technically
> consistent behavior though.
>
> I don't want to rush a decision now, I think I need to gather more user
> feedback after the 9.4 release. I'll ask Bastien to revert that commit
> and put this off to 9.5, rather than writing an updated NEWS entry like
> he asked.
>
> Either way, we'll need to update Worg to clearly document what's going
> on here.
>
[-- Attachment #2: Type: text/html, Size: 3531 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] (Tiny) Tweak Python session null return value
2020-02-17 19:45 ` John Kitchin
@ 2020-02-17 20:45 ` Jack Kamm
2020-02-17 22:27 ` John Kitchin
0 siblings, 1 reply; 9+ messages in thread
From: Jack Kamm @ 2020-02-17 20:45 UTC (permalink / raw
To: John Kitchin; +Cc: Bastien, org-mode-email
Hi John,
John Kitchin <jkitchin@andrew.cmu.edu> writes:
> I can see why you would want to see True/False there, but to get the value,
> you need to specifically return what you want because AFAIK the body is
> wrapped in a function that is evaluated to get the value, it is not simply
> the last thing that gets evaluated.
This is true for non-session blocks, which require explicitly calling
"return". However, session blocks aren't wrapped in functions and don't
use "return" (even before the most recent patches). The problem is that
variables created in a function have local scope, so session blocks
can't be wrapped in functions.
> Your example clarified to me at least why it would be tricky to figure
> it out, you can't rely on the last line, for example.
Since the recent patches, we do extract the last line, using the Python
ast module, however this only works if the last line is a top-level
statement like "f()" or "1+1", not an assignment (like "x = 1+1") or an
indented block (like "if:...else:...").
> I don't know if there is some special Python variable that contains
> that.
There actually is -- in most Python interpreters, the variable "_"
(underscore) refers to the last statement, unless it's been explicitly
assigned to. This is what was previously relied on. Unfortunately, using
"_" for a dummy variable is a common Python idiom (e.g. "for _ in
range(10)"), and if used would break all subsequent Python session
blocks. So we no longer rely on "_".
In the standard Python interpreter, we can also use "__builtins__._",
but this doesn't work in IPython. Furthermore, this only works for code
explicitly entered in the shell, it won't work for code executed in
"exec()" or "eval()", which we now rely on, because it handles
indentation much more robustly. In particular, ob-python sessions have
had longstanding issues with multiline indented blocks, which are now
solved in the recent patches.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] (Tiny) Tweak Python session null return value
2020-02-17 20:45 ` Jack Kamm
@ 2020-02-17 22:27 ` John Kitchin
0 siblings, 0 replies; 9+ messages in thread
From: John Kitchin @ 2020-02-17 22:27 UTC (permalink / raw
To: Jack Kamm; +Cc: Bastien, org-mode-email
[-- Attachment #1: Type: text/plain, Size: 2861 bytes --]
On Mon, Feb 17, 2020 at 3:46 PM Jack Kamm <jackkamm@gmail.com> wrote:
> Hi John,
>
> John Kitchin <jkitchin@andrew.cmu.edu> writes:
>
> > I can see why you would want to see True/False there, but to get the
> value,
> > you need to specifically return what you want because AFAIK the body is
> > wrapped in a function that is evaluated to get the value, it is not
> simply
> > the last thing that gets evaluated.
>
> This is true for non-session blocks, which require explicitly calling
> "return". However, session blocks aren't wrapped in functions and don't
> use "return" (even before the most recent patches). The problem is that
> variables created in a function have local scope, so session blocks
> can't be wrapped in functions.
>
Fair point, I am not a python session user (I have used the ob-ipython for
a long time, or stand-alone python blocks), and I had forgotten or not
known of this. Indeed in a REPL, you get something closer to what you
originally suggested.
>>> a = 1
>>> if a:
... True
... else:
... False
...
True
I guess I would expect something like that if I was using a Python session
in org-mode. It is like a REPL that is easier to edit.
My earlier concern is mostly related to consistency of what an org Python
block does compared to what you might do at a REPL or from a script. I also
note that I almost never use :results value, and almost always prefer
:results output. That reflects the kind of stuff we usually do here though,
and may not be representative of others.
> > Your example clarified to me at least why it would be tricky to figure
> > it out, you can't rely on the last line, for example.
>
> Since the recent patches, we do extract the last line, using the Python
> ast module, however this only works if the last line is a top-level
> statement like "f()" or "1+1", not an assignment (like "x = 1+1") or an
> indented block (like "if:...else:...").
>
> > I don't know if there is some special Python variable that contains
> > that.
>
> There actually is -- in most Python interpreters, the variable "_"
> (underscore) refers to the last statement, unless it's been explicitly
> assigned to. This is what was previously relied on. Unfortunately, using
> "_" for a dummy variable is a common Python idiom (e.g. "for _ in
> range(10)"), and if used would break all subsequent Python session
> blocks. So we no longer rely on "_".
> In the standard Python interpreter, we can also use "__builtins__._",
> but this doesn't work in IPython. Furthermore, this only works for code
explicitly entered in the shell, it won't work for code executed in
> "exec()" or "eval()", which we now rely on, because it handles
> indentation much more robustly. In particular, ob-python sessions have
> had longstanding issues with multiline indented blocks, which are now
> solved in the recent patches.
>
[-- Attachment #2: Type: text/html, Size: 6289 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-02-17 22:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-17 16:24 [PATCH] (Tiny) Tweak Python session null return value Jack Kamm
2020-02-17 17:42 ` Bastien
2020-02-17 19:07 ` Jack Kamm
2020-02-17 22:53 ` Bastien
2020-02-17 17:52 ` John Kitchin
2020-02-17 19:05 ` Jack Kamm
2020-02-17 19:45 ` John Kitchin
2020-02-17 20:45 ` Jack Kamm
2020-02-17 22:27 ` John Kitchin
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.