unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: master e32c7d2: Change Python eval to send directly instead of using temporary files
       [not found] ` <20210903122829.EAAC220B71@vcs0.savannah.gnu.org>
@ 2021-09-03 23:04   ` Stefan Monnier
  2021-09-04  9:49     ` bug#49822: " Augusto Stoffel
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2021-09-03 23:04 UTC (permalink / raw)
  To: emacs-devel; +Cc: Augusto Stoffel

>     Change Python eval to send directly instead of using temporary files

FWIW, sending large amounts of text via pty can be troublesome (some
OSes have been known to insert additional control chars every 256
bytes, or to do weird things after 4096 bytes leading the send to never
complete, ...), which is why it's very common for comint modes to send
regions of text via temp files.


        Stefan




^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-03 23:04   ` master e32c7d2: Change Python eval to send directly instead of using temporary files Stefan Monnier
@ 2021-09-04  9:49     ` Augusto Stoffel
  2021-09-04 13:52       ` Stefan Monnier
                         ` (3 more replies)
  0 siblings, 4 replies; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-04  9:49 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 49822, emacs-devel

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

On Fri,  3 Sep 2021 at 19:04, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>>     Change Python eval to send directly instead of using temporary files
>
> FWIW, sending large amounts of text via pty can be troublesome (some
> OSes have been known to insert additional control chars every 256
> bytes, or to do weird things after 4096 bytes leading the send to never
> complete, ...), which is why it's very common for comint modes to send
> regions of text via temp files.
>
>
>         Stefan

Okay, the 4096 bytes limit indeed exists in IPython (but not in the
standard Python interpreters, apparently).

I've attached a patch that reverts to using temporary files for
sufficiently long strings (would this magic 4096 ever require
customization?).  The patch also solves bug#32042, which is mostly
unrelated.

I would like pass text inline as much as possible because the
back-and-forth of temp files is pretty slow over Tramp, which makes the
Python shell rather annoying to use.

As to some OSes inserting additional control characters every 256 bytes,
I've never encountered this, but it seems a rather tricky problem to
work around.  Before this commit, the "plumbing code" sent to the
interpreter could already be above 256 bytes (e.g., if the generated
temp file names are modestly long).  Inserting newline characters at
strategic places would print random prompt strings, and therefore also
introduce complications.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fixes-for-python-shell-send-string-and-python-shell-.patch --]
[-- Type: text/x-patch, Size: 3852 bytes --]

From 6fdbecb66d4409e2ce03c559861138966abb4acd Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 4 Sep 2021 11:16:11 +0200
Subject: [PATCH] Fixes for 'python-shell-send-string' and
 'python-shell-send-file'

* lisp/progmodes/python.el (python-shell-send-string): use a temporary
file for sufficiently long strings.
(python-shell-send-file, python-shell-eval-file-setup-code): Avoid
showing "plumbing code" in the traceback (bug#32042).
---
 lisp/progmodes/python.el | 46 +++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 306cc3a542..d8ec032402 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3127,12 +3127,18 @@ python-shell-send-string
 t when called interactively."
   (interactive
    (list (read-string "Python command: ") nil t))
-  (comint-send-string
-   (or process (python-shell-get-process-or-error msg))
-   (format "exec(%s);__PYTHON_EL_eval(%s, %s)\n"
-           (python-shell--encode-string python-shell-eval-setup-code)
-           (python-shell--encode-string string)
-           (python-shell--encode-string (or (buffer-file-name) "<string>")))))
+  (let ((process (or process (python-shell-get-process-or-error msg)))
+        (code (format "exec(%s);__PYTHON_EL_eval(%s, %s)\n"
+                      (python-shell--encode-string python-shell-eval-setup-code)
+                      (python-shell--encode-string string)
+                      (python-shell--encode-string (or (buffer-file-name)
+                                                       "<string>")))))
+    (if (<= (string-bytes code) 4096)
+        (comint-send-string process code)
+      (let* ((temp-file-name (with-current-buffer (process-buffer process)
+                               (python-shell--save-temp-file string)))
+             (file-name (or (buffer-file-name) temp-file-name)))
+        (python-shell-send-file file-name process temp-file-name t)))))
 
 (defvar python-shell-output-filter-in-progress nil)
 (defvar python-shell-output-filter-buffer nil)
@@ -3372,6 +3378,18 @@ python-shell-send-defun
        nil ;; noop
        msg))))
 
+
+(defconst python-shell-eval-file-setup-code
+  "\
+def __PYTHON_EL_eval_file(filename, tempname, encoding, delete):
+    import codecs, os
+    with codecs.open(tempname or filename, encoding=encoding) as file:
+        source = file.read().encode(encoding)
+    if delete and tempname:
+        os.remove(tempname)
+    return __PYTHON_EL_eval(source, filename)"
+  "Code used to evaluate files in inferior Python processes.")
+
 (defun python-shell-send-file (file-name &optional process temp-file-name
                                          delete msg)
   "Send FILE-NAME to inferior Python PROCESS.
@@ -3401,15 +3419,13 @@ python-shell-send-file
     (comint-send-string
      process
      (format
-      (concat
-       "import codecs, os;"
-       "__pyfile = codecs.open('''%s''', encoding='''%s''');"
-       "__code = __pyfile.read().encode('''%s''');"
-       "__pyfile.close();"
-       (when (and delete temp-file-name)
-         (format "os.remove('''%s''');" temp-file-name))
-       "exec(compile(__code, '''%s''', 'exec'));")
-      (or temp-file-name file-name) encoding encoding file-name))))
+      "exec(%s);exec(%s);__PYTHON_EL_eval_file(%s, %s, %s, %s)\n"
+      (python-shell--encode-string python-shell-eval-setup-code)
+      (python-shell--encode-string python-shell-eval-file-setup-code)
+      (python-shell--encode-string file-name)
+      (python-shell--encode-string (or temp-file-name ""))
+      (python-shell--encode-string (symbol-name encoding))
+      (if delete "True" "False")))))
 
 (defun python-shell-switch-to-shell (&optional msg)
   "Switch to inferior Python process buffer.
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-04  9:49     ` bug#49822: " Augusto Stoffel
@ 2021-09-04 13:52       ` Stefan Monnier
  2021-09-05  6:13       ` Barton, Mark
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 34+ messages in thread
From: Stefan Monnier @ 2021-09-04 13:52 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: emacs-devel, 49822

> As to some OSes inserting additional control characters every 256 bytes,
> I've never encountered this, but it seems a rather tricky problem to
> work around.

IIRC this was on some version of macos, and maybe the 256B limit applied
only to text without newlines or something like that [it was too long ago].


        Stefan




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-04  9:49     ` bug#49822: " Augusto Stoffel
  2021-09-04 13:52       ` Stefan Monnier
@ 2021-09-05  6:13       ` Barton, Mark
  2021-09-05  7:46         ` bug#49822: " Augusto Stoffel
  2021-09-05  8:10         ` Augusto Stoffel
  2021-09-05  7:41       ` bug#49822: " Lars Ingebrigtsen
  2021-09-05 16:36       ` Andreas Schwab
  3 siblings, 2 replies; 34+ messages in thread
From: Barton, Mark @ 2021-09-05  6:13 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: 49822@debbugs.gnu.org, Stefan Monnier, emacs-devel@gnu.org

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


On Sep 4, 2021, at 2:49 AM, Augusto Stoffel <arstoffel@gmail.com<mailto:arstoffel@gmail.com>> wrote:

On Fri,  3 Sep 2021 at 19:04, Stefan Monnier <monnier@iro.umontreal.ca<mailto:monnier@iro.umontreal.ca>> wrote:

   Change Python eval to send directly instead of using temporary files

FWIW, sending large amounts of text via pty can be troublesome (some
OSes have been known to insert additional control chars every 256
bytes, or to do weird things after 4096 bytes leading the send to never
complete, ...), which is why it's very common for comint modes to send
regions of text via temp files.


       Stefan

Okay, the 4096 bytes limit indeed exists in IPython (but not in the
standard Python interpreters, apparently).

I've attached a patch that reverts to using temporary files for
sufficiently long strings (would this magic 4096 ever require
customization?).  The patch also solves bug#32042, which is mostly
unrelated.

I would like pass text inline as much as possible because the
back-and-forth of temp files is pretty slow over Tramp, which makes the
Python shell rather annoying to use.

As to some OSes inserting additional control characters every 256 bytes,
I've never encountered this, but it seems a rather tricky problem to
work around.  Before this commit, the "plumbing code" sent to the
interpreter could already be above 256 bytes (e.g., if the generated
temp file names are modestly long).  Inserting newline characters at
strategic places would print random prompt strings, and therefore also
introduce complications.

<0001-Fixes-for-python-shell-send-string-and-python-shell-.patch>

Today I was trying to export my monthly org document that uses org babel python blocks to produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d - Change Python eval to send directly instead of using temporary files.

Below I show the python session buffer, first where it is working and then where it is broken for me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.

In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t understand the problem. Any ideas on what other information I can gather that would be useful?


Working session buffer when using previous python.el to export document

Python 3.9.6 (default, Jun 29 2021, 05:25:02)
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> org_babel_python_eoe
>>> python.el: native completion setup loaded
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>>

Broken session buffer with current python.el in e32c7d2a8d

Python 3.9.6 (default, Jun 29 2021, 05:25:02)
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>>
org_babel_python_eoe
>>>   File "<stdin>", line 1
    exec("def __PYTHON_EL_eval(source, filename):\n    import ast, sys\n    if sys.version_info[0] == 2:\n        from __builtin__ import compile, eval, globals\n    else:\n        from builtins import compile, eval, globals\n    sys.stdout.write('\\n')\n    try:\n        p, e = ast.parse(source, filename), None\n    except SyntaxError:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)\n        return\n    if p.body and isinstance(p.body[-1], ast.Expr):\n        e = p.body.pop()\n    try:\n        g = globals()\n        exec(compile(p, filename, 'exec'), g, g)\n        if e:\n            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)\n    except Exception:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)");__PYTHON_EL_eval("\ndef __PYTHON_EL_native_completion_setup():\n    try:\n        import readline\n\n        try:\n            import __builtin__\n        except ImportError:\n            # Python 3\n            import builtins as __bexec("def __PYTHON_EL_eval(source, filename):\n    import ast, sys\n    if sys.version_info[0] == 2:\n        from __builtin__ import compile, eval, globals\n    else:\n        from builtins import compile, eval, globals\n    sys.stdout.write('\\n')\n    try:\n        p, e = ast.parse(source, filename), None\n    except SyntaxError:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)\n        return\n    if p.body and isinstance(p.body[-1], ast.Expr):\n        e = p.body.pop()\n    try:\n        g = globals()\n        exec(compile(p, filename, 'exec'), g, g)\n        if e:\n            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)\n    except Exception:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)");__PYTHON_EL_eval("try:\n    import ast\n    with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-VMz5YY/python-Xmy5oT') as __org_babel_python_tmpfile:\n        __org_babel_python_ast = ast.parse(__org_babel_python_tmpfile.read())\n    __org_babel_python_final = __org_babel_python_ast.body[-1]\n    if isinstance(__org_babel_python_final, ast.Expr):\n        __org_babel_python_ast.body = __org_babel_python_ast.body[:-1]\n        exec(compile(__org_babel_python_ast, '<string>', 'exec'))\n        __org_babel_python_final = eval(compile(ast.Expression(\n            __org_babel_python_final.value), '<string>', 'eval'))\n        with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-VMz5YY/python-hzPBKk', 'w') as __org_babel_python_tmpfile:\n            if False:\n                import pprint\n                __org_babel_python_tmpfile.write(pprint.pformat(__org_babel_python_final))\n            else:\n                __org_babel_python_tmpfile.write(str(__org_babel_python_final))\n    else:\n        exec(compile(__org_babel_python_ast, '<string>', 'exec'))\n        __org_babel_python_final = None\nexcept:\n    raise\nfinally:\n    print('org_babel_python_eoe')", "<string>")
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                          ^
SyntaxError: invalid syntax
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 3, in <module>
NameError: name 'dfMembership' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfMembership' is not defined
>>>
2021-08-24org_babel_python_eoe
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfYPTdata' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfTrainingRpt' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfScoutbookScoutsRB' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 3, in <module>
NameError: name 'dfCompareSBwithTM' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 1, in <module>
NameError: name 'dfAdvData' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 1, in <module>
NameError: name 'dfScoutbookScoutsRB' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 1, in <module>
NameError: name 'dfCompareSBwithTM' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 3, in <module>
NameError: name 'dfScoutbookScoutsRB' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 22, in <module>
NameError: name 'dfAdvData' is not defined
>>>


[-- Attachment #2: Type: text/html, Size: 50519 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-04  9:49     ` bug#49822: " Augusto Stoffel
  2021-09-04 13:52       ` Stefan Monnier
  2021-09-05  6:13       ` Barton, Mark
@ 2021-09-05  7:41       ` Lars Ingebrigtsen
  2021-09-05 16:36       ` Andreas Schwab
  3 siblings, 0 replies; 34+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-05  7:41 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 49822, 32042, Stefan Monnier, emacs-devel

Augusto Stoffel <arstoffel@gmail.com> writes:

> I've attached a patch that reverts to using temporary files for
> sufficiently long strings (would this magic 4096 ever require
> customization?).  The patch also solves bug#32042, which is mostly
> unrelated.

Thanks; pushed to Emacs 28 (and I guess I'll close bug#32042, then).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-05  6:13       ` Barton, Mark
@ 2021-09-05  7:46         ` Augusto Stoffel
  2021-09-05  8:10         ` Augusto Stoffel
  1 sibling, 0 replies; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-05  7:46 UTC (permalink / raw)
  To: Barton, Mark; +Cc: 49822@debbugs.gnu.org, Stefan Monnier, emacs-devel@gnu.org

On Sun,  5 Sep 2021 at 06:13, "Barton, Mark" <Mark.Barton@disney.com> wrote:

> Today I was trying to export my monthly org document that uses org babel python blocks to
> produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d
> - Change Python eval to send directly instead of using temporary files.
>
> Below I show the python session buffer, first where it is working and then where it is broken for
> me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.
>
> In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t
> understand the problem. Any ideas on what other information I can gather that would be
> useful?

I can run all the examples in
https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-python.html
after the changes of e32c7d2a8d.

Can you provide a small example to reproduce the problem you encountered?





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-05  6:13       ` Barton, Mark
  2021-09-05  7:46         ` bug#49822: " Augusto Stoffel
@ 2021-09-05  8:10         ` Augusto Stoffel
  2021-09-05 17:18           ` bug#49822: " Mark Barton
  1 sibling, 1 reply; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-05  8:10 UTC (permalink / raw)
  To: Barton, Mark; +Cc: 49822@debbugs.gnu.org, Stefan Monnier, emacs-devel@gnu.org

On Sun,  5 Sep 2021 at 06:13, "Barton, Mark" <Mark.Barton@disney.com> wrote:

> Today I was trying to export my monthly org document that uses org babel python blocks to
> produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d
> - Change Python eval to send directly instead of using temporary files.
>
> Below I show the python session buffer, first where it is working and then where it is broken for
> me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.
>
> In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t
> understand the problem. Any ideas on what other information I can gather that would be
> useful?

Two more things you could try:

1) set `python-shell-completion-native-enable' to nil
2) pull the current master, after commit 1fdd898704

I'd be curious to see the effect of 1) before doing 2), if you have the
time for this quick test.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-04  9:49     ` bug#49822: " Augusto Stoffel
                         ` (2 preceding siblings ...)
  2021-09-05  7:41       ` bug#49822: " Lars Ingebrigtsen
@ 2021-09-05 16:36       ` Andreas Schwab
  2021-09-05 18:40         ` Augusto Stoffel
  3 siblings, 1 reply; 34+ messages in thread
From: Andreas Schwab @ 2021-09-05 16:36 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 49822, Stefan Monnier, emacs-devel

On Sep 04 2021, Augusto Stoffel wrote:

> I've attached a patch that reverts to using temporary files for
> sufficiently long strings (would this magic 4096 ever require
> customization?).

On BSD-like systems, the limit is 1024.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-05  8:10         ` Augusto Stoffel
@ 2021-09-05 17:18           ` Mark Barton
  2021-09-05 17:33             ` Mark Barton
  2021-09-05 17:46             ` Augusto Stoffel
  0 siblings, 2 replies; 34+ messages in thread
From: Mark Barton @ 2021-09-05 17:18 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: 49822@debbugs.gnu.org, Stefan Monnier, emacs-devel@gnu.org

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



> On Sep 5, 2021, at 1:10 AM, Augusto Stoffel <arstoffel@gmail.com> wrote:
> 
> On Sun,  5 Sep 2021 at 06:13, "Barton, Mark" <Mark.Barton@disney.com> wrote:
> 
>> Today I was trying to export my monthly org document that uses org babel python blocks to
>> produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d
>> - Change Python eval to send directly instead of using temporary files.
>> 
>> Below I show the python session buffer, first where it is working and then where it is broken for
>> me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.
>> 
>> In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t
>> understand the problem. Any ideas on what other information I can gather that would be
>> useful?
> 
> Two more things you could try:
> 
> 1) set `python-shell-completion-native-enable' to nil
> 2) pull the current master, after commit 1fdd898704
> 
> I'd be curious to see the effect of 1) before doing 2), if you have the
> time for this quick test.


I created a test org and csv file to test. Before I added the inline call to the test file, changing the python-shell-completion-native-enable to nil did get the my def pd2org to work. Otherwise it would work on the second export if I did not kill the python-chain session. The inline call to file_date returns an error “Inline error: multiline result cannot be used”

The attached pdf shows the successful export when I use the previous python.el.

I just compiled from master at commit c5b654b3f1

Recent commits
c5b654b3f1 master origin/master Autoload cl-struct-slot-info
7c7a47b86e ; * etc/NEWS: Fix a recent change.
3d0276e98b Improve the documentation around the read-key/minibuffer prompting
73a90cda4a Clarify completion-list-mode NEWS entry
8f2e8add98 ; * doc/emacs/maintaining.texi (Looking Up Identifiers): Fix last change.
0972cbe42f * lisp/progmodes/xref.el: Fix defcustoms (bug#50067)
2ed2999ce5 Improve documentation of new Xref options
1fdd898704 Fixes for 'python-shell-send-string' and 'python-shell-send-file'
ba84ec8bd9 Fix error handling in 'ispell-init-process'
e6f936eb4d Cleanup tramp-tests.el

Setting the python-shell-completion-native-enable to nil cleans up the session buffer python-chain

Python 3.9.6 (default, Jun 29 2021, 05:25:02) 
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> 
org_babel_python_eoe
>>> 
2021-08-24org_babel_python_eoe
>>> 
org_babel_python_eoe
>>> 
org_babel_python_eoe
>>> 
2021-09-05org_babel_python_eoe
>>> 

But I now get the error that prevents the pdf export: “Inline error: multiline result cannot be used"

[-- Attachment #2.1: Type: text/html, Size: 4383 bytes --]

[-- Attachment #2.2: ts_org_babel_python.csv --]
[-- Type: text/csv, Size: 23 bytes --]

1, one
2, two
3, three

[-- Attachment #2.3: Type: text/html, Size: 197 bytes --]

[-- Attachment #2.4: ts_org_babel_python.org --]
[-- Type: application/octet-stream, Size: 1782 bytes --]

#+TITLE: Test Org Babel Export with Python Blocks
#+AUTHOR: Mark Barton
#+DATE: 2021-09-05
#+OPTIONS: toc:nil H:4 num:nil ^:nil
#+EXCLUDE_TAGS: ignore
#+LATEX_CLASS: article
#+LaTeX_HEADER: \usepackage[letterpaper,margin=0.5in, bottom=1in]{geometry}
#+latex_header: \usepackage{float}
#+LaTeX_HEADER: \usepackage{enumitem}\setlist[itemize]{nosep}
#+LaTeX_HEADER: \usepackage{needspace}
#+LaTeX_HEADER: \usepackage{booktabs}
#+LaTeX_HEADER: \usepackage{xcolor}
#+LaTeX_HEADER: \let\OldRule\rule
#+LaTeX_HEADER: \renewcommand{\rule}[2]{\OldRule{\linewidth}{#2}}
#+LaTeX_HEADER: \setlength{\parindent}{0pt}
#+LaTeX_HEADER: \setlength{\parskip}{12pt}
#+PROPERTY: header-args:python :session python-chain :exports results :results raw replace

      
*** Initialize python                                              :ignore:
#+BEGIN_SRC python
import pandas as pd
from tabulate import tabulate

#+END_SRC

#+RESULTS:

#+NAME: file_date
#+begin_src python :var file="Roster_Report.csv" :exports none
import os
import time
print(time.strftime("%Y-%m-%d",time.localtime(os.path.getmtime(file))), end="")
#+end_src


*** Process downloaded csv files with Python Pandas for custom reports :ignore:
#+begin_src python :exports none
def pd2org(dataframe):
    return tabulate(dataframe, headers="keys", tablefmt="orgtbl", showindex="always")


dfTest = pd.read_csv("./ts_org_babel_python.csv")
#+end_src

*** This is a test                                                 :ignore:
#+NAME: Test_Table
#+begin_src python 
pd2org(dfTest)

#+end_src

*** Summary
Include multiple source blocks and an inline call to create data for document.

#+ATTR_LATEX: :placement [H]
#+caption: Test Table call_file_date[:results output :results raw](file="./ts_org_babel_python.csv")
#+RESULTS: Test_Table


[-- Attachment #2.5: Type: text/html, Size: 197 bytes --]

[-- Attachment #2.6: ts_org_babel_python.pdf --]
[-- Type: application/pdf, Size: 58501 bytes --]

[-- Attachment #2.7: Type: text/html, Size: 206 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-05 17:18           ` bug#49822: " Mark Barton
@ 2021-09-05 17:33             ` Mark Barton
  2021-09-05 17:46             ` Augusto Stoffel
  1 sibling, 0 replies; 34+ messages in thread
From: Mark Barton @ 2021-09-05 17:33 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: 49822@debbugs.gnu.org, Stefan Monnier, emacs-devel@gnu.org

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



> On Sep 5, 2021, at 10:18 AM, Mark Barton <mbarton98@gmail.com> wrote:
> 
> 
> 
>> On Sep 5, 2021, at 1:10 AM, Augusto Stoffel <arstoffel@gmail.com <mailto:arstoffel@gmail.com>> wrote:
>> 
>> On Sun,  5 Sep 2021 at 06:13, "Barton, Mark" <Mark.Barton@disney.com <mailto:Mark.Barton@disney.com>> wrote:
>> 
>>> Today I was trying to export my monthly org document that uses org babel python blocks to
>>> produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d
>>> - Change Python eval to send directly instead of using temporary files.
>>> 
>>> Below I show the python session buffer, first where it is working and then where it is broken for
>>> me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.
>>> 
>>> In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t
>>> understand the problem. Any ideas on what other information I can gather that would be
>>> useful?
>> 
>> Two more things you could try:
>> 
>> 1) set `python-shell-completion-native-enable' to nil
>> 2) pull the current master, after commit 1fdd898704
>> 
>> I'd be curious to see the effect of 1) before doing 2), if you have the
>> time for this quick test.
> 
> 
> I created a test org and csv file to test. Before I added the inline call to the test file, changing the python-shell-completion-native-enable to nil did get the my def pd2org to work. Otherwise it would work on the second export if I did not kill the python-chain session. The inline call to file_date returns an error “Inline error: multiline result cannot be used”
> 
> The attached pdf shows the successful export when I use the previous python.el.
> 
> I just compiled from master at commit c5b654b3f1
> 
> Recent commits
> c5b654b3f1 master origin/master Autoload cl-struct-slot-info
> 7c7a47b86e ; * etc/NEWS: Fix a recent change.
> 3d0276e98b Improve the documentation around the read-key/minibuffer prompting
> 73a90cda4a Clarify completion-list-mode NEWS entry
> 8f2e8add98 ; * doc/emacs/maintaining.texi (Looking Up Identifiers): Fix last change.
> 0972cbe42f * lisp/progmodes/xref.el: Fix defcustoms (bug#50067)
> 2ed2999ce5 Improve documentation of new Xref options
> 1fdd898704 Fixes for 'python-shell-send-string' and 'python-shell-send-file'
> ba84ec8bd9 Fix error handling in 'ispell-init-process'
> e6f936eb4d Cleanup tramp-tests.el
> 
> Setting the python-shell-completion-native-enable to nil cleans up the session buffer python-chain
> 
> Python 3.9.6 (default, Jun 29 2021, 05:25:02) 
> [Clang 12.0.5 (clang-1205.0.22.9)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> 
> org_babel_python_eoe
> >>> 
> 2021-08-24org_babel_python_eoe
> >>> 
> org_babel_python_eoe
> >>> 
> org_babel_python_eoe
> >>> 
> 2021-09-05org_babel_python_eoe
> >>> 
> 
> But I now get the error that prevents the pdf export: “Inline error: multiline result cannot be used"
> <ts_org_babel_python.csv>
> <ts_org_babel_python.org>
> <ts_org_babel_python.pdf>

Here is a simpler one to attempt to reproduce. If I comment out the #+PROPERTY line to not use a session then it works. With the #+PROPERTY line, I get the “Inline error: multiline result cannot be used”.


#+TITLE: Test Org Babel Export with Python Blocks
#+AUTHOR: Mark Barton
#+DATE: 2021-09-05
#+OPTIONS: toc:nil H:4 num:nil ^:nil
#+EXCLUDE_TAGS: ignore
#+LATEX_CLASS: article
#+LaTeX_HEADER: \usepackage[letterpaper,margin=0.5in, bottom=1in]{geometry}
#+latex_header: \usepackage{float}
#+LaTeX_HEADER: \usepackage{enumitem}\setlist[itemize]{nosep}
#+LaTeX_HEADER: \usepackage{needspace}
#+LaTeX_HEADER: \usepackage{booktabs}
#+LaTeX_HEADER: \usepackage{xcolor}
#+LaTeX_HEADER: \let\OldRule\rule
#+LaTeX_HEADER: \renewcommand{\rule}[2]{\OldRule{\linewidth}{#2}}
#+LaTeX_HEADER: \setlength{\parindent}{0pt}
#+LaTeX_HEADER: \setlength{\parskip}{12pt}
#+PROPERTY: header-args:python :session python-chain :exports results :results raw replace

      
*** Initialize python                                              :ignore:

#+NAME: file_date
#+begin_src python :var file="Roster_Report.csv" :exports none
import os
import time
print(time.strftime("%Y-%m-%d",time.localtime(os.path.getmtime(file))), end="")
#+end_src


*** Summary

Test inline call_file_date[:results output :results raw](file="./ts_org_babel_python.csv")


[-- Attachment #2: Type: text/html, Size: 8078 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-05 17:18           ` bug#49822: " Mark Barton
  2021-09-05 17:33             ` Mark Barton
@ 2021-09-05 17:46             ` Augusto Stoffel
  1 sibling, 0 replies; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-05 17:46 UTC (permalink / raw)
  To: Mark Barton; +Cc: 49822@debbugs.gnu.org, Stefan Monnier, emacs-devel@gnu.org

On Sun,  5 Sep 2021 at 10:18, Mark Barton <mbarton98@gmail.com> wrote:

> I created a test org and csv file to test. Before I added the inline call to the test file, changing
> the python-shell-completion-native-enable to nil did get the my def pd2org to work. Otherwise
> it would work on the second export if I did not kill the python-chain session. The inline call to
> file_date returns an error “Inline error: multiline result cannot be
> used”

Then I'm pretty sure this problem has to do with line length limits in
the TTY, as Stefan had already alerted.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-05 16:36       ` Andreas Schwab
@ 2021-09-05 18:40         ` Augusto Stoffel
  2021-09-06  7:43           ` Michael Albinus
  0 siblings, 1 reply; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-05 18:40 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 49822, Stefan Monnier, emacs-devel

On Sun,  5 Sep 2021 at 18:36, Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Sep 04 2021, Augusto Stoffel wrote:
>
>> I've attached a patch that reverts to using temporary files for
>> sufficiently long strings (would this magic 4096 ever require
>> customization?).
>
> On BSD-like systems, the limit is 1024.
>
> Andreas.

Okay, so is the TTY line length limit of the OS available in Lisp
somewhere?  Otherwise, can we at least trust that 1024 is the universal
lower bound?





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-05 18:40         ` Augusto Stoffel
@ 2021-09-06  7:43           ` Michael Albinus
  2021-09-06  8:40             ` Andreas Schwab
  0 siblings, 1 reply; 34+ messages in thread
From: Michael Albinus @ 2021-09-06  7:43 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 49822, Andreas Schwab, Stefan Monnier, emacs-devel

Augusto Stoffel <arstoffel@gmail.com> writes:

> Okay, so is the TTY line length limit of the OS available in Lisp
> somewhere?  Otherwise, can we at least trust that 1024 is the universal
> lower bound?

On systems with a POSIX shell:

--8<---------------cut here---------------start------------->8---
(let ((default-directory "/"))
  (read (shell-command-to-string "getconf LINE_MAX")))
--8<---------------cut here---------------end--------------->8---

Note, that this can differ when you connect to remote hosts. That's why
I have bound default-directory to a local file name in the example.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-06  7:43           ` Michael Albinus
@ 2021-09-06  8:40             ` Andreas Schwab
  2021-09-06 11:23               ` Michael Albinus
  0 siblings, 1 reply; 34+ messages in thread
From: Andreas Schwab @ 2021-09-06  8:40 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 49822, Augusto Stoffel, Stefan Monnier, emacs-devel

On Sep 06 2021, Michael Albinus wrote:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> Okay, so is the TTY line length limit of the OS available in Lisp
>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>> lower bound?
>
> On systems with a POSIX shell:
>
> (let ((default-directory "/"))
>   (read (shell-command-to-string "getconf LINE_MAX")))

LINE_MAX has nothing to do with the tty line discipline.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-06  8:40             ` Andreas Schwab
@ 2021-09-06 11:23               ` Michael Albinus
  2021-09-06 11:53                 ` Andreas Schwab
  0 siblings, 1 reply; 34+ messages in thread
From: Michael Albinus @ 2021-09-06 11:23 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 49822, Augusto Stoffel, Stefan Monnier, emacs-devel

Andreas Schwab <schwab@linux-m68k.org> writes:

>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>> lower bound?
>>
>> On systems with a POSIX shell:
>>
>> (let ((default-directory "/"))
>>   (read (shell-command-to-string "getconf LINE_MAX")))
>
> LINE_MAX has nothing to do with the tty line discipline.

I'll never learn it :-(

We've discussed this issue some years ago already. IIRC, the outcome was
to use "getconf PIPE_BUF /" instead.

> Andreas.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-06 11:23               ` Michael Albinus
@ 2021-09-06 11:53                 ` Andreas Schwab
  2021-09-06 12:00                   ` Michael Albinus
                                     ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Andreas Schwab @ 2021-09-06 11:53 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 49822, Augusto Stoffel, Stefan Monnier, emacs-devel

On Sep 06 2021, Michael Albinus wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>>> lower bound?
>>>
>>> On systems with a POSIX shell:
>>>
>>> (let ((default-directory "/"))
>>>   (read (shell-command-to-string "getconf LINE_MAX")))
>>
>> LINE_MAX has nothing to do with the tty line discipline.
>
> I'll never learn it :-(
>
> We've discussed this issue some years ago already. IIRC, the outcome was
> to use "getconf PIPE_BUF /" instead.

Neither has PIPE_BUF.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-06 11:53                 ` Andreas Schwab
@ 2021-09-06 12:00                   ` Michael Albinus
  2021-09-06 16:08                     ` Augusto Stoffel
       [not found]                   ` <855f9eaf1b1d39bb9325d0a88e7f66c0ba0b45d0.camel@gmail.com>
  2021-09-07 17:37                   ` Augusto Stoffel
  2 siblings, 1 reply; 34+ messages in thread
From: Michael Albinus @ 2021-09-06 12:00 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 49822, Augusto Stoffel, Stefan Monnier, emacs-devel

Andreas Schwab <schwab@linux-m68k.org> writes:

> Neither has PIPE_BUF.

Great that you always say what's not working. Perhaps you have a better
answer to Augusto?

> Andreas.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-06 12:00                   ` Michael Albinus
@ 2021-09-06 16:08                     ` Augusto Stoffel
  0 siblings, 0 replies; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-06 16:08 UTC (permalink / raw)
  To: Michael Albinus
  Cc: 49822, Lars Ingebrigtsen, Andreas Schwab, Stefan Monnier,
	emacs-devel

On Mon,  6 Sep 2021 at 14:00, Michael Albinus <michael.albinus@gmx.de> wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Neither has PIPE_BUF.
>
> Great that you always say what's not working. Perhaps you have a better
> answer to Augusto?
>
>> Andreas.
>
> Best regards, Michael.

Thanks for the ideas.  If there's no simple and platform-independent
way to obtain the line length limit in a PTY, then I think there's no
reason to attempt anything fancy in python.el (or any derived comint
mode, for that matter).

So the options seem to be: either hard-code a limit of 1024 chars, or
create a defcustom with a platform-dependent default.  But such a
defcustom should probably be part of comint.el, and should be added by
someone who (unlike me) actually understands this stuff.

Working around a limit of less than 1024 characters is too awkward, so I
hope this is a safe assumption.  The juggling of temp files that existed
in python.el for many years already required lines of ~350 chars,
depending on how the OS names temp files.

Sorry for introducing a bug in master.  I think I got it fixed here, and
I'll submit the patch when I'm confident about it.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
       [not found]                   ` <855f9eaf1b1d39bb9325d0a88e7f66c0ba0b45d0.camel@gmail.com>
@ 2021-09-06 22:15                     ` Andy Moreton
  2021-09-07  7:18                       ` Andreas Schwab
  0 siblings, 1 reply; 34+ messages in thread
From: Andy Moreton @ 2021-09-06 22:15 UTC (permalink / raw)
  To: 49822; +Cc: emacs-devel

On Mon 06 Sep 2021, Andreas Schwab wrote:

> On Sep 06 2021, Michael Albinus wrote:
>
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>
>>>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>>>> lower bound?
>>>>
>>>> On systems with a POSIX shell:
>>>>
>>>> (let ((default-directory "/"))
>>>>   (read (shell-command-to-string "getconf LINE_MAX")))
>>>
>>> LINE_MAX has nothing to do with the tty line discipline.
>>
>> I'll never learn it :-(
>>
>> We've discussed this issue some years ago already. IIRC, the outcome was
>> to use "getconf PIPE_BUF /" instead.
>
> Neither has PIPE_BUF.
>
> Andreas.

Why not stop being so unhelpful and actually share your knowledge of
the correct answer ?

What precisely is the limitation, and how can it be discovered ?

Please explain.

    AndyM






^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-06 22:15                     ` Andy Moreton
@ 2021-09-07  7:18                       ` Andreas Schwab
  0 siblings, 0 replies; 34+ messages in thread
From: Andreas Schwab @ 2021-09-07  7:18 UTC (permalink / raw)
  To: Andy Moreton; +Cc: 49822, emacs-devel

On Sep 06 2021, Andy Moreton wrote:

> On Mon 06 Sep 2021, Andreas Schwab wrote:
>
>> On Sep 06 2021, Michael Albinus wrote:
>>
>>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>>
>>>>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>>>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>>>>> lower bound?
>>>>>
>>>>> On systems with a POSIX shell:
>>>>>
>>>>> (let ((default-directory "/"))
>>>>>   (read (shell-command-to-string "getconf LINE_MAX")))
>>>>
>>>> LINE_MAX has nothing to do with the tty line discipline.
>>>
>>> I'll never learn it :-(
>>>
>>> We've discussed this issue some years ago already. IIRC, the outcome was
>>> to use "getconf PIPE_BUF /" instead.
>>
>> Neither has PIPE_BUF.
>>
>> Andreas.
>
> Why not stop being so unhelpful and actually share your knowledge of
> the correct answer ?

Why not stop being so unhelpful and actually share your knowledge of the
correct answer?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-06 11:53                 ` Andreas Schwab
  2021-09-06 12:00                   ` Michael Albinus
       [not found]                   ` <855f9eaf1b1d39bb9325d0a88e7f66c0ba0b45d0.camel@gmail.com>
@ 2021-09-07 17:37                   ` Augusto Stoffel
  2021-09-07 17:48                     ` Eli Zaretskii
                                       ` (2 more replies)
  2 siblings, 3 replies; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-07 17:37 UTC (permalink / raw)
  To: Barton, Mark
  Cc: Andy Moreton, emacs-devel, Andreas Schwab, Stefan Monnier, 49822,
	Michael Albinus, Lars Ingebrigtsen

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

On Mon,  6 Sep 2021 at 13:53, Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Sep 06 2021, Michael Albinus wrote:
>
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>
>>>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>>>> lower bound?
>>>>
>>>> On systems with a POSIX shell:
>>>>
>>>> (let ((default-directory "/"))
>>>>   (read (shell-command-to-string "getconf LINE_MAX")))
>>>
>>> LINE_MAX has nothing to do with the tty line discipline.
>>
>> I'll never learn it :-(
>>
>> We've discussed this issue some years ago already. IIRC, the outcome was
>> to use "getconf PIPE_BUF /" instead.
>
> Neither has PIPE_BUF.
>
> Andreas.

Okay then. Since there seem to be no better alternatives, I have
attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
some day someone adds a variable specifying a more precise limit, then
we can change this.

I have also rearranged things a bit so that the setup code is sent to
the inferior process just once, rather than of on every call to
`python-shell-send-string'.  This way, the smaller line length limit
doesn't increase too much the use of temp files, which, as I mentioned,
is slow over ssh.

It would be great if someone with access to a BSD-like OS could test
this.  I can only test locally on Linux and over ssh between Linux
machines.

PS: I have some more suggestions around the Python shell.  Is the ideal
workflow to keep creating bugs with a small patch to each improvement,
or do you prefer to review a larger collection of changes bundled
together?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Reduce-limit-of-line-length-sent-to-the-Python-infer.patch --]
[-- Type: text/x-patch, Size: 3680 bytes --]

From 05947ad82219af3f4bcb228f076995f181a7255d Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Mon, 6 Sep 2021 23:34:48 +0200
Subject: [PATCH] Reduce limit of line length sent to the Python inferior

* lisp/progmodes/python.el
(python-shell-comint-watch-for-first-prompt-output-filter): Send setup
code to the inferior process only once and at this stage.
(python-shell-send-string-no-output): Revert changes of e32c7d2a8d
(python-shell-send-string): Assume a smaller TTY line length limit,
as required by some OSes.
(python-shell-send-file): Ditto.
---
 lisp/progmodes/python.el | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d8ec032402..347c3ffe00 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2826,6 +2826,15 @@ python-shell-comint-watch-for-first-prompt-output-filter
           (setq python-shell--first-prompt-received-output-buffer nil)
         (setq-local python-shell--first-prompt-received t)
         (setq python-shell--first-prompt-received-output-buffer nil)
+        (cl-letf (((symbol-function 'python-shell-send-string)
+                   (lambda (string process)
+                     (comint-send-string
+                      process
+                      (format "exec(%s)\n" (python-shell--encode-string string))))))
+          ;; Bootstrap: the normal definition of `python-shell-send-string'
+          ;; depends on the Python code sent here.
+          (python-shell-send-string-no-output python-shell-eval-setup-code)
+          (python-shell-send-string-no-output python-shell-eval-file-setup-code))
         (with-current-buffer (current-buffer)
           (let ((inhibit-quit nil))
             (run-hooks 'python-shell-first-prompt-hook))))))
@@ -3128,12 +3137,11 @@ python-shell-send-string
   (interactive
    (list (read-string "Python command: ") nil t))
   (let ((process (or process (python-shell-get-process-or-error msg)))
-        (code (format "exec(%s);__PYTHON_EL_eval(%s, %s)\n"
-                      (python-shell--encode-string python-shell-eval-setup-code)
+        (code (format "__PYTHON_EL_eval(%s, %s)\n"
                       (python-shell--encode-string string)
                       (python-shell--encode-string (or (buffer-file-name)
                                                        "<string>")))))
-    (if (<= (string-bytes code) 4096)
+    (if (<= (string-bytes code) 1024)
         (comint-send-string process code)
       (let* ((temp-file-name (with-current-buffer (process-buffer process)
                                (python-shell--save-temp-file string)))
@@ -3180,8 +3188,7 @@ python-shell-send-string-no-output
         (inhibit-quit t))
     (or
      (with-local-quit
-       (comint-send-string
-        process (format "exec(%s)\n" (python-shell--encode-string string)))
+       (python-shell-send-string string process)
        (while python-shell-output-filter-in-progress
          ;; `python-shell-output-filter' takes care of setting
          ;; `python-shell-output-filter-in-progress' to NIL after it
@@ -3419,9 +3426,7 @@ python-shell-send-file
     (comint-send-string
      process
      (format
-      "exec(%s);exec(%s);__PYTHON_EL_eval_file(%s, %s, %s, %s)\n"
-      (python-shell--encode-string python-shell-eval-setup-code)
-      (python-shell--encode-string python-shell-eval-file-setup-code)
+      "__PYTHON_EL_eval_file(%s, %s, %s, %s)\n"
       (python-shell--encode-string file-name)
       (python-shell--encode-string (or temp-file-name ""))
       (python-shell--encode-string (symbol-name encoding))
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 17:37                   ` Augusto Stoffel
@ 2021-09-07 17:48                     ` Eli Zaretskii
  2021-09-07 17:59                       ` Augusto Stoffel
  2021-09-07 18:13                       ` Augusto Stoffel
  2021-09-08  3:17                     ` Barton, Mark
  2021-09-08  7:50                     ` Lars Ingebrigtsen
  2 siblings, 2 replies; 34+ messages in thread
From: Eli Zaretskii @ 2021-09-07 17:48 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: andrewjmoreton, Mark.Barton, emacs-devel, schwab, monnier, 49822,
	michael.albinus, larsi

> From: Augusto Stoffel <arstoffel@gmail.com>
> Date: Tue, 07 Sep 2021 19:37:41 +0200
> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
>  Andreas Schwab <schwab@linux-m68k.org>,
>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
> 
> Okay then. Since there seem to be no better alternatives, I have
> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
> some day someone adds a variable specifying a more precise limit, then
> we can change this.

I thought the conclusion was that we do know the limits, but they are
different on different OSes?  If so, why not use the correct limit for
each OS, instead of using the most strict limit?  The underlying OS is
available in system-type.

> -    (if (<= (string-bytes code) 4096)
> +    (if (<= (string-bytes code) 1024)

In any case, IMO this should be a defconst with a suitable doc string,
so that we'd have this stuff documented for posterity.

Thanks.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 17:48                     ` Eli Zaretskii
@ 2021-09-07 17:59                       ` Augusto Stoffel
  2021-09-07 18:19                         ` Eli Zaretskii
  2021-09-07 18:13                       ` Augusto Stoffel
  1 sibling, 1 reply; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-07 17:59 UTC (permalink / raw)
  To: Eli Zaretskii
  Cc: andrewjmoreton, Mark.Barton, emacs-devel, schwab, monnier, 49822,
	michael.albinus, larsi

On Tue,  7 Sep 2021 at 20:48, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Date: Tue, 07 Sep 2021 19:37:41 +0200
>> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
>>  Andreas Schwab <schwab@linux-m68k.org>,
>>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
>>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
>> 
>> Okay then. Since there seem to be no better alternatives, I have
>> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
>> some day someone adds a variable specifying a more precise limit, then
>> we can change this.
>
> I thought the conclusion was that we do know the limits, but they are
> different on different OSes?  If so, why not use the correct limit for
> each OS, instead of using the most strict limit?  The underlying OS is
> available in system-type.
>
>> -    (if (<= (string-bytes code) 4096)
>> +    (if (<= (string-bytes code) 1024)
>
> In any case, IMO this should be a defconst with a suitable doc string,
> so that we'd have this stuff documented for posterity.
>
> Thanks.

Yes, I agree this should be a defconst, defined in comint.el or maybe
even in C, if there turns out to be a sensible way to query the OS.

I'm afraid I've already proved that I am not up to this particular task :-)



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 17:48                     ` Eli Zaretskii
  2021-09-07 17:59                       ` Augusto Stoffel
@ 2021-09-07 18:13                       ` Augusto Stoffel
  2021-09-07 18:31                         ` Eli Zaretskii
  1 sibling, 1 reply; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-07 18:13 UTC (permalink / raw)
  To: Eli Zaretskii
  Cc: andrewjmoreton, Mark.Barton, emacs-devel, schwab, monnier, 49822,
	michael.albinus, larsi

On Tue,  7 Sep 2021 at 20:48, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Date: Tue, 07 Sep 2021 19:37:41 +0200
>> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
>>  Andreas Schwab <schwab@linux-m68k.org>,
>>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
>>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
>> 
>> Okay then. Since there seem to be no better alternatives, I have
>> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
>> some day someone adds a variable specifying a more precise limit, then
>> we can change this.
>
> I thought the conclusion was that we do know the limits, but they are
> different on different OSes?  If so, why not use the correct limit for
> each OS, instead of using the most strict limit?  The underlying OS is
> available in system-type.
>
>> -    (if (<= (string-bytes code) 4096)
>> +    (if (<= (string-bytes code) 1024)
>
> In any case, IMO this should be a defconst with a suitable doc string,
> so that we'd have this stuff documented for posterity.
>
> Thanks.

Another open question (at least to me) is what happens when you put an
SSH connection in the game.  Presumably it has some buffer limits as
well, and this seems even harder to determine exactly.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 17:59                       ` Augusto Stoffel
@ 2021-09-07 18:19                         ` Eli Zaretskii
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii @ 2021-09-07 18:19 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: andrewjmoreton, Mark.Barton, emacs-devel, schwab, monnier, 49822,
	michael.albinus, larsi

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: Mark.Barton@disney.com,  andrewjmoreton@gmail.com,  emacs-devel@gnu.org,
>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>   michael.albinus@gmx.de,  larsi@gnus.org
> Date: Tue, 07 Sep 2021 19:59:06 +0200
> 
> On Tue,  7 Sep 2021 at 20:48, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> From: Augusto Stoffel <arstoffel@gmail.com>
> >> Date: Tue, 07 Sep 2021 19:37:41 +0200
> >> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
> >>  Andreas Schwab <schwab@linux-m68k.org>,
> >>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
> >>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
> >> 
> >> Okay then. Since there seem to be no better alternatives, I have
> >> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
> >> some day someone adds a variable specifying a more precise limit, then
> >> we can change this.
> >
> > I thought the conclusion was that we do know the limits, but they are
> > different on different OSes?  If so, why not use the correct limit for
> > each OS, instead of using the most strict limit?  The underlying OS is
> > available in system-type.
> >
> >> -    (if (<= (string-bytes code) 4096)
> >> +    (if (<= (string-bytes code) 1024)
> >
> > In any case, IMO this should be a defconst with a suitable doc string,
> > so that we'd have this stuff documented for posterity.
> >
> > Thanks.
> 
> Yes, I agree this should be a defconst, defined in comint.el or maybe
> even in C, if there turns out to be a sensible way to query the OS.
> 
> I'm afraid I've already proved that I am not up to this particular task :-)

I didn't mean to say anything like that, not at all.  Mine was a minor
comment, it's supposed to be easy to augment your patch to avoid those
minor issues.  Nothing complicated or fancy is expected, just a
defconst with a value conditioned on system-type.

As for where should that defconst live, I personally have no problem
with its living in python.el for now.  We can always move it later if
the need arises.

Thanks.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 18:13                       ` Augusto Stoffel
@ 2021-09-07 18:31                         ` Eli Zaretskii
  2021-09-07 19:00                           ` Augusto Stoffel
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2021-09-07 18:31 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: andrewjmoreton, Mark.Barton, emacs-devel, schwab, monnier, 49822,
	michael.albinus, larsi

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: andrewjmoreton@gmail.com,  Mark.Barton@disney.com,  emacs-devel@gnu.org,
>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>   michael.albinus@gmx.de,  larsi@gnus.org
> Date: Tue, 07 Sep 2021 20:13:36 +0200
> 
> >> -    (if (<= (string-bytes code) 4096)
> >> +    (if (<= (string-bytes code) 1024)
> >
> > In any case, IMO this should be a defconst with a suitable doc string,
> > so that we'd have this stuff documented for posterity.
> >
> > Thanks.
> 
> Another open question (at least to me) is what happens when you put an
> SSH connection in the game.  Presumably it has some buffer limits as
> well, and this seems even harder to determine exactly.

SSH connection between what and what?



^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 18:31                         ` Eli Zaretskii
@ 2021-09-07 19:00                           ` Augusto Stoffel
  2021-09-07 19:16                             ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-07 19:00 UTC (permalink / raw)
  To: Eli Zaretskii
  Cc: andrewjmoreton, Mark.Barton, emacs-devel, schwab, monnier, 49822,
	michael.albinus, larsi

On Tue,  7 Sep 2021 at 21:31, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Cc: andrewjmoreton@gmail.com,  Mark.Barton@disney.com,  emacs-devel@gnu.org,
>>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>>   michael.albinus@gmx.de,  larsi@gnus.org
>> Date: Tue, 07 Sep 2021 20:13:36 +0200
>> 
>> >> -    (if (<= (string-bytes code) 4096)
>> >> +    (if (<= (string-bytes code) 1024)
>> >
>> > In any case, IMO this should be a defconst with a suitable doc string,
>> > so that we'd have this stuff documented for posterity.
>> >
>> > Thanks.
>> 
>> Another open question (at least to me) is what happens when you put an
>> SSH connection in the game.  Presumably it has some buffer limits as
>> well, and this seems even harder to determine exactly.
>
> SSH connection between what and what?

I'm sitting at machine A but my inferior process is running on
machine B and the machines talk via SSH.

There are several steps in the communication here: Emacs -> SSH client
-> SSH server -> inferior process.  Each step has potential limitations.
If the SSH server talks to the inferior process via a PTY (I'm just
pondering, and I don't know if this is usual setup or not), then we
would have to take machine B's PTY restrictions into account, which
would defeat the purpose of knowing the exact characteristics of machine
A's PTYs.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 19:00                           ` Augusto Stoffel
@ 2021-09-07 19:16                             ` Eli Zaretskii
  2021-09-08  7:02                               ` Michael Albinus
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2021-09-07 19:16 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: andrewjmoreton, Mark.Barton, emacs-devel, schwab, monnier, 49822,
	michael.albinus, larsi

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: andrewjmoreton@gmail.com,  Mark.Barton@disney.com,  emacs-devel@gnu.org,
>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>   michael.albinus@gmx.de,  larsi@gnus.org
> Date: Tue, 07 Sep 2021 21:00:28 +0200
> 
> >> Another open question (at least to me) is what happens when you put an
> >> SSH connection in the game.  Presumably it has some buffer limits as
> >> well, and this seems even harder to determine exactly.
> >
> > SSH connection between what and what?
> 
> I'm sitting at machine A but my inferior process is running on
> machine B and the machines talk via SSH.

You mean, you run a remote subprocess via Tramp?

> There are several steps in the communication here: Emacs -> SSH client
> -> SSH server -> inferior process.  Each step has potential limitations.
> If the SSH server talks to the inferior process via a PTY (I'm just
> pondering, and I don't know if this is usual setup or not), then we
> would have to take machine B's PTY restrictions into account, which
> would defeat the purpose of knowing the exact characteristics of machine
> A's PTYs.

I don't see how B's PTY restrictions are relevant, but maybe Micheal
will (assuming you are talking about remote subprocesses).



^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 17:37                   ` Augusto Stoffel
  2021-09-07 17:48                     ` Eli Zaretskii
@ 2021-09-08  3:17                     ` Barton, Mark
  2021-09-08  5:09                       ` Augusto Stoffel
  2021-09-08  7:50                     ` Lars Ingebrigtsen
  2 siblings, 1 reply; 34+ messages in thread
From: Barton, Mark @ 2021-09-08  3:17 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: Andy Moreton, emacs-devel@gnu.org, Andreas Schwab, Stefan Monnier,
	49822@debbugs.gnu.org, Michael Albinus, Lars Ingebrigtsen

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



On Sep 7, 2021, at 10:37 AM, Augusto Stoffel <arstoffel@gmail.com<mailto:arstoffel@gmail.com>> wrote:

On Mon,  6 Sep 2021 at 13:53, Andreas Schwab <schwab@linux-m68k.org<mailto:schwab@linux-m68k.org>> wrote:

On Sep 06 2021, Michael Albinus wrote:

Andreas Schwab <schwab@linux-m68k.org<mailto:schwab@linux-m68k.org>> writes:

Okay, so is the TTY line length limit of the OS available in Lisp
somewhere?  Otherwise, can we at least trust that 1024 is the universal
lower bound?

On systems with a POSIX shell:

(let ((default-directory "/"))
 (read (shell-command-to-string "getconf LINE_MAX")))

LINE_MAX has nothing to do with the tty line discipline.

I'll never learn it :-(

We've discussed this issue some years ago already. IIRC, the outcome was
to use "getconf PIPE_BUF /" instead.

Neither has PIPE_BUF.

Andreas.

Okay then. Since there seem to be no better alternatives, I have
attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
some day someone adds a variable specifying a more precise limit, then
we can change this.

I have also rearranged things a bit so that the setup code is sent to
the inferior process just once, rather than of on every call to
`python-shell-send-string'.  This way, the smaller line length limit
doesn't increase too much the use of temp files, which, as I mentioned,
is slow over ssh.

It would be great if someone with access to a BSD-like OS could test
this.  I can only test locally on Linux and over ssh between Linux
machines.

PS: I have some more suggestions around the Python shell.  Is the ideal
workflow to keep creating bugs with a small patch to each improvement,
or do you prefer to review a larger collection of changes bundled
together?

<0001-Reduce-limit-of-line-length-sent-to-the-Python-infer.patch>

I tested the patch, 0001-Reduce-limit-of-line-length-sent-to-the-Python-infer.patch, on macOS 11.5.1 and although my minimal example runs, my original document using org babel session with multiple python blocks is failing with the message in the mini buffer “Inline error: multiline result cannot be used”. I can try some lower value than 1024 and report back, but below is what I saw with attempting to use the debugger to see what the string looks like in my use case.

Setting debug-on-entry to comint-send-string and running a src block, I get:

Debugger entered--returning value: nil
  comint-send-string(#<process filedate> "__PYTHON_EL_eval_file(\"/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/pybQ7oBh\", \"/var/folders/kf/...")
  python-shell-send-file("/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/p..." #<process filedate> "/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/p..." t)
  python-shell-send-string("try:\n    import ast\n    with open('/var/folders/kf...")
  org-babel-python--send-string("*filedate*" "import ast\nwith open('/var/folders/kf/zdpzgs9d30b3...")
  org-babel-python-evaluate-session("*filedate*" "file=\"Roster_Report.csv\"\nimport os\nimport time\npri..." value ("raw" "replace"))
  org-babel-python-evaluate("*filedate*" "file=\"Roster_Report.csv\"\nimport os\nimport time\npri..." value ("raw" "replace") nil)
  org-babel-execute:python("import os\nimport time\nprint(time.strftime(\"%Y-%m-%..." ((:var file . "Roster_Report.csv") (:colname-names) (:rowname-names) (:result-params "raw" "replace") (:result-type . value) (:results . "raw replace") (:exports . "results") (:cache . "no") (:noweb . "no") (:hlines . "no") (:async . "yes") (:tangle . "reports.py") (:comments . "both") (:session . "filedate")))
  org-babel-execute-src-block(nil ("python" "import os\nimport time\nprint(time.strftime(\"%Y-%m-%..." ((:var file . "Roster_Report.csv") (:colname-names) (:rowname-names) (:result-params "replace" "raw") (:result-type . value) (:results . "replace raw") (:exports . "results") (:session . "filedate") (:comments . "both") (:tangle . "reports.py") (:async . "yes") (:hlines . "no") (:noweb . "no") (:cache . "no")) "" "file_date" 7735 "(ref:%s)"))
  org-ctrl-c-ctrl-c(nil)
  funcall-interactively(org-ctrl-c-ctrl-c nil)
  command-execute(org-ctrl-c-ctrl-c)

If I look at the file passed, pybQ7oBh, the contents looks like it will open another file.

try:
    import ast
    with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-K4MbU2/python-hNII4l') as __org_babel_python_tmpfile:
        __org_babel_python_ast = ast.parse(__org_babel_python_tmpfile.read())
    __org_babel_python_final = __org_babel_python_ast.body[-1]
    if isinstance(__org_babel_python_final, ast.Expr):
        __org_babel_python_ast.body = __org_babel_python_ast.body[:-1]
        exec(compile(__org_babel_python_ast, '<string>', 'exec'))
        __org_babel_python_final = eval(compile(ast.Expression(
            __org_babel_python_final.value), '<string>', 'eval'))
        with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-K4MbU2/python-ra4ysF', 'w') as __org_babel_python_tmpfile:
            if False:
                import pprint
                __org_babel_python_tmpfile.write(pprint.pformat(__org_babel_python_final))
            else:
                __org_babel_python_tmpfile.write(str(__org_babel_python_final))
    else:
        exec(compile(__org_babel_python_ast, '<string>', 'exec'))
        __org_babel_python_final = None
except:
    raise
finally:
    print('org_babel_python_eoe')

[-- Attachment #2: Type: text/html, Size: 10818 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-08  3:17                     ` Barton, Mark
@ 2021-09-08  5:09                       ` Augusto Stoffel
  0 siblings, 0 replies; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-08  5:09 UTC (permalink / raw)
  To: Barton, Mark
  Cc: Andy Moreton, emacs-devel@gnu.org, Andreas Schwab, Stefan Monnier,
	49822@debbugs.gnu.org, Michael Albinus, Lars Ingebrigtsen

On Wed,  8 Sep 2021 at 03:17, "Barton, Mark" <Mark.Barton@disney.com> wrote:

> I tested the patch, 0001-Reduce-limit-of-line-length-sent-to-the-Python-infer.patch, on
> macOS 11.5.1 and although my minimal example runs, my original document using org babel
> session with multiple python blocks is failing with the message in the mini buffer “Inline error:
> multiline result cannot be used”. I can try some lower value than 1024 and report back, but
> below is what I saw with attempting to use the debugger to see what the string looks like in my
> use case.

This looks like a different matter, and the patch below might solve it.
But I think it would make sense to open a new bug for this point, to we
get a chance of getting through with the TTY line limit discussion in
this thread.

diff --git a/lisp/org/ob-python.el b/lisp/org/ob-python.el
index 7911205d08..e3714964b7 100644
--- a/lisp/org/ob-python.el
+++ b/lisp/org/ob-python.el
@@ -352,7 +352,7 @@ org-babel-python--send-string
 		   org-babel-python-eoe-indicator
 		   string-buffer))
 	(accept-process-output (get-buffer-process (current-buffer))))
-      (org-babel-chomp (substring string-buffer 0 (match-beginning 0))))))
+      (org-babel-chomp (substring string-buffer 1 (match-beginning 0))))))
 
 (defun org-babel-python-evaluate-session
     (session body &optional result-type result-params)





^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 19:16                             ` Eli Zaretskii
@ 2021-09-08  7:02                               ` Michael Albinus
  0 siblings, 0 replies; 34+ messages in thread
From: Michael Albinus @ 2021-09-08  7:02 UTC (permalink / raw)
  To: Eli Zaretskii
  Cc: andrewjmoreton, Mark.Barton, emacs-devel, schwab, monnier, 49822,
	larsi, Augusto Stoffel

Eli Zaretskii <eliz@gnu.org> writes:

Hi,

>> I'm sitting at machine A but my inferior process is running on
>> machine B and the machines talk via SSH.
>
> You mean, you run a remote subprocess via Tramp?
>
>> There are several steps in the communication here: Emacs -> SSH client
>> -> SSH server -> inferior process.  Each step has potential limitations.
>> If the SSH server talks to the inferior process via a PTY (I'm just
>> pondering, and I don't know if this is usual setup or not), then we
>> would have to take machine B's PTY restrictions into account, which
>> would defeat the purpose of knowing the exact characteristics of machine
>> A's PTYs.
>
> I don't see how B's PTY restrictions are relevant, but maybe Micheal
> will (assuming you are talking about remote subprocesses).

Tramp tries to handle such cases. But of course, a respective test
wouldn't hurt. It could be on the machine which has shown the
limitations, accessing it via "/ssh::".

Best regards, Michael.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-07 17:37                   ` Augusto Stoffel
  2021-09-07 17:48                     ` Eli Zaretskii
  2021-09-08  3:17                     ` Barton, Mark
@ 2021-09-08  7:50                     ` Lars Ingebrigtsen
  2021-09-08 14:05                       ` Augusto Stoffel
  2 siblings, 1 reply; 34+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-08  7:50 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: Andy Moreton, Barton, Mark, emacs-devel, Andreas Schwab,
	Stefan Monnier, 49822, Michael Albinus

Augusto Stoffel <arstoffel@gmail.com> writes:

> I have also rearranged things a bit so that the setup code is sent to
> the inferior process just once, rather than of on every call to
> `python-shell-send-string'.  This way, the smaller line length limit
> doesn't increase too much the use of temp files, which, as I mentioned,
> is slow over ssh.

This leads to compilation warnings about
python-shell-eval-file-setup-code etc.

Can you adjust the patch to fix that, and also add the defconst for
1024, like Eli discussed?

> PS: I have some more suggestions around the Python shell.  Is the ideal
> workflow to keep creating bugs with a small patch to each improvement,
> or do you prefer to review a larger collection of changes bundled
> together?

Smaller patches are fine by me, and you can submit them with `M-x
submit-emacs-patch'.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-08  7:50                     ` Lars Ingebrigtsen
@ 2021-09-08 14:05                       ` Augusto Stoffel
  2021-09-09 13:48                         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 34+ messages in thread
From: Augusto Stoffel @ 2021-09-08 14:05 UTC (permalink / raw)
  To: Lars Ingebrigtsen
  Cc: Andy Moreton, Barton, Mark, emacs-devel, Andreas Schwab,
	Stefan Monnier, 49822, Michael Albinus

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

On Wed,  8 Sep 2021 at 09:50, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> I have also rearranged things a bit so that the setup code is sent to
>> the inferior process just once, rather than of on every call to
>> `python-shell-send-string'.  This way, the smaller line length limit
>> doesn't increase too much the use of temp files, which, as I mentioned,
>> is slow over ssh.
>
> This leads to compilation warnings about
> python-shell-eval-file-setup-code etc.
>
> Can you adjust the patch to fix that, and also add the defconst for
> 1024, like Eli discussed?
>
>> PS: I have some more suggestions around the Python shell.  Is the ideal
>> workflow to keep creating bugs with a small patch to each improvement,
>> or do you prefer to review a larger collection of changes bundled
>> together?
>
> Smaller patches are fine by me, and you can submit them with `M-x
> submit-emacs-patch'.

Hi Lars,

I've attached a new patch addressing these 2 points.

I've added the max line length defconst to comint.el, since this
something general about PTYs.  The value reflects my knowledge of the
situation (4096 on Linux, possibly 1024 on other OSes).  Hopefully other
people will fill in more cases eventually.

FWIW, when I read the man page for termios, it seems pretty clear that
MAX_CANON is the name of this parameter we're discussing here.  But if I
do 'getconf MAX_CANON /', I get 255, which is definitely not the right
value for my system.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Better-treatment-of-line-length-limits-for-the-Pytho.patch --]
[-- Type: text/x-patch, Size: 8015 bytes --]

From 3eab275464f4a53bf86153f5f3693e522a240b28 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Mon, 6 Sep 2021 23:34:48 +0200
Subject: [PATCH] Better treatment of line length limits for the Python
 inferior

* lisp/comint.el (comint-max-line-length): new constant reflecting a
safe maximum line size that can be sent to an inferior process.
* lisp/progmodes/python.el
(python-shell-comint-watch-for-first-prompt-output-filter): Send setup
code to the inferior process only once and at this stage.
(python-shell-eval-setup-code, python-shell-eval-file-setup-code):
Move, unchanged, to an earlier point to avoid byte-compiler warnings.
(python-shell-send-string-no-output): Revert changes of e32c7d2a8d
(python-shell-send-string): Use 'comint-max-line-length' to decide
when to resort to temp files.
(python-shell-send-string, python-shell-send-file): Don't send setup
code each time.
---
 lisp/comint.el           |  6 +++
 lisp/progmodes/python.el | 98 +++++++++++++++++++++-------------------
 2 files changed, 57 insertions(+), 47 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index e058e6b8cf..c2e528a5d5 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -479,6 +479,12 @@ comint-terminfo-terminal
   :group 'comint
   :version "26.1")
 
+(defconst comint-max-line-length
+  (pcase system-type
+    ('gnu/linux 4096)
+    (_ 1024))
+  "Maximum line length, in bytes, accepted by the inferior process.")
+
 (defvar comint-mode-map
   (let ((map (make-sparse-keymap)))
     ;; Keys:
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d8ec032402..4f222b4cf5 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2811,6 +2811,44 @@ python-shell-first-prompt-hook
   :type 'hook
   :group 'python)
 
+(defconst python-shell-eval-setup-code
+  "\
+def __PYTHON_EL_eval(source, filename):
+    import ast, sys
+    if sys.version_info[0] == 2:
+        from __builtin__ import compile, eval, globals
+    else:
+        from builtins import compile, eval, globals
+    sys.stdout.write('\\n')
+    try:
+        p, e = ast.parse(source, filename), None
+    except SyntaxError:
+        t, v, tb = sys.exc_info()
+        sys.excepthook(t, v, tb.tb_next)
+        return
+    if p.body and isinstance(p.body[-1], ast.Expr):
+        e = p.body.pop()
+    try:
+        g = globals()
+        exec(compile(p, filename, 'exec'), g, g)
+        if e:
+            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)
+    except Exception:
+        t, v, tb = sys.exc_info()
+        sys.excepthook(t, v, tb.tb_next)"
+  "Code used to evaluate statements in inferior Python processes.")
+
+(defconst python-shell-eval-file-setup-code
+  "\
+def __PYTHON_EL_eval_file(filename, tempname, encoding, delete):
+    import codecs, os
+    with codecs.open(tempname or filename, encoding=encoding) as file:
+        source = file.read().encode(encoding)
+    if delete and tempname:
+        os.remove(tempname)
+    return __PYTHON_EL_eval(source, filename)"
+  "Code used to evaluate files in inferior Python processes.")
+
 (defun python-shell-comint-watch-for-first-prompt-output-filter (output)
   "Run `python-shell-first-prompt-hook' when first prompt is found in OUTPUT."
   (when (not python-shell--first-prompt-received)
@@ -2826,6 +2864,15 @@ python-shell-comint-watch-for-first-prompt-output-filter
           (setq python-shell--first-prompt-received-output-buffer nil)
         (setq-local python-shell--first-prompt-received t)
         (setq python-shell--first-prompt-received-output-buffer nil)
+        (cl-letf (((symbol-function 'python-shell-send-string)
+                   (lambda (string process)
+                     (comint-send-string
+                      process
+                      (format "exec(%s)\n" (python-shell--encode-string string))))))
+          ;; Bootstrap: the normal definition of `python-shell-send-string'
+          ;; depends on the Python code sent here.
+          (python-shell-send-string-no-output python-shell-eval-setup-code)
+          (python-shell-send-string-no-output python-shell-eval-file-setup-code))
         (with-current-buffer (current-buffer)
           (let ((inhibit-quit nil))
             (run-hooks 'python-shell-first-prompt-hook))))))
@@ -3081,33 +3128,6 @@ python-shell--save-temp-file
       (delete-trailing-whitespace))
     temp-file-name))
 
-(defconst python-shell-eval-setup-code
-  "\
-def __PYTHON_EL_eval(source, filename):
-    import ast, sys
-    if sys.version_info[0] == 2:
-        from __builtin__ import compile, eval, globals
-    else:
-        from builtins import compile, eval, globals
-    sys.stdout.write('\\n')
-    try:
-        p, e = ast.parse(source, filename), None
-    except SyntaxError:
-        t, v, tb = sys.exc_info()
-        sys.excepthook(t, v, tb.tb_next)
-        return
-    if p.body and isinstance(p.body[-1], ast.Expr):
-        e = p.body.pop()
-    try:
-        g = globals()
-        exec(compile(p, filename, 'exec'), g, g)
-        if e:
-            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)
-    except Exception:
-        t, v, tb = sys.exc_info()
-        sys.excepthook(t, v, tb.tb_next)"
-  "Code used to evaluate statements in inferior Python processes.")
-
 (defalias 'python-shell--encode-string
   (let ((fun (if (and (fboundp 'json-serialize)
                       (>= emacs-major-version 28))
@@ -3128,12 +3148,11 @@ python-shell-send-string
   (interactive
    (list (read-string "Python command: ") nil t))
   (let ((process (or process (python-shell-get-process-or-error msg)))
-        (code (format "exec(%s);__PYTHON_EL_eval(%s, %s)\n"
-                      (python-shell--encode-string python-shell-eval-setup-code)
+        (code (format "__PYTHON_EL_eval(%s, %s)\n"
                       (python-shell--encode-string string)
                       (python-shell--encode-string (or (buffer-file-name)
                                                        "<string>")))))
-    (if (<= (string-bytes code) 4096)
+    (if (<= (string-bytes code) comint-max-line-length)
         (comint-send-string process code)
       (let* ((temp-file-name (with-current-buffer (process-buffer process)
                                (python-shell--save-temp-file string)))
@@ -3180,8 +3199,7 @@ python-shell-send-string-no-output
         (inhibit-quit t))
     (or
      (with-local-quit
-       (comint-send-string
-        process (format "exec(%s)\n" (python-shell--encode-string string)))
+       (python-shell-send-string string process)
        (while python-shell-output-filter-in-progress
          ;; `python-shell-output-filter' takes care of setting
          ;; `python-shell-output-filter-in-progress' to NIL after it
@@ -3378,18 +3396,6 @@ python-shell-send-defun
        nil ;; noop
        msg))))
 
-
-(defconst python-shell-eval-file-setup-code
-  "\
-def __PYTHON_EL_eval_file(filename, tempname, encoding, delete):
-    import codecs, os
-    with codecs.open(tempname or filename, encoding=encoding) as file:
-        source = file.read().encode(encoding)
-    if delete and tempname:
-        os.remove(tempname)
-    return __PYTHON_EL_eval(source, filename)"
-  "Code used to evaluate files in inferior Python processes.")
-
 (defun python-shell-send-file (file-name &optional process temp-file-name
                                          delete msg)
   "Send FILE-NAME to inferior Python PROCESS.
@@ -3419,9 +3425,7 @@ python-shell-send-file
     (comint-send-string
      process
      (format
-      "exec(%s);exec(%s);__PYTHON_EL_eval_file(%s, %s, %s, %s)\n"
-      (python-shell--encode-string python-shell-eval-setup-code)
-      (python-shell--encode-string python-shell-eval-file-setup-code)
+      "__PYTHON_EL_eval_file(%s, %s, %s, %s)\n"
       (python-shell--encode-string file-name)
       (python-shell--encode-string (or temp-file-name ""))
       (python-shell--encode-string (symbol-name encoding))
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files
  2021-09-08 14:05                       ` Augusto Stoffel
@ 2021-09-09 13:48                         ` Lars Ingebrigtsen
  0 siblings, 0 replies; 34+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-09 13:48 UTC (permalink / raw)
  To: Augusto Stoffel
  Cc: Andy Moreton, Barton, Mark, emacs-devel, Andreas Schwab,
	Stefan Monnier, 49822, Michael Albinus

Augusto Stoffel <arstoffel@gmail.com> writes:

> I've attached a new patch addressing these 2 points.

Thanks; applied to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2021-09-09 13:48 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210903122828.16890.65271@vcs0.savannah.gnu.org>
     [not found] ` <20210903122829.EAAC220B71@vcs0.savannah.gnu.org>
2021-09-03 23:04   ` master e32c7d2: Change Python eval to send directly instead of using temporary files Stefan Monnier
2021-09-04  9:49     ` bug#49822: " Augusto Stoffel
2021-09-04 13:52       ` Stefan Monnier
2021-09-05  6:13       ` Barton, Mark
2021-09-05  7:46         ` bug#49822: " Augusto Stoffel
2021-09-05  8:10         ` Augusto Stoffel
2021-09-05 17:18           ` bug#49822: " Mark Barton
2021-09-05 17:33             ` Mark Barton
2021-09-05 17:46             ` Augusto Stoffel
2021-09-05  7:41       ` bug#49822: " Lars Ingebrigtsen
2021-09-05 16:36       ` Andreas Schwab
2021-09-05 18:40         ` Augusto Stoffel
2021-09-06  7:43           ` Michael Albinus
2021-09-06  8:40             ` Andreas Schwab
2021-09-06 11:23               ` Michael Albinus
2021-09-06 11:53                 ` Andreas Schwab
2021-09-06 12:00                   ` Michael Albinus
2021-09-06 16:08                     ` Augusto Stoffel
     [not found]                   ` <855f9eaf1b1d39bb9325d0a88e7f66c0ba0b45d0.camel@gmail.com>
2021-09-06 22:15                     ` Andy Moreton
2021-09-07  7:18                       ` Andreas Schwab
2021-09-07 17:37                   ` Augusto Stoffel
2021-09-07 17:48                     ` Eli Zaretskii
2021-09-07 17:59                       ` Augusto Stoffel
2021-09-07 18:19                         ` Eli Zaretskii
2021-09-07 18:13                       ` Augusto Stoffel
2021-09-07 18:31                         ` Eli Zaretskii
2021-09-07 19:00                           ` Augusto Stoffel
2021-09-07 19:16                             ` Eli Zaretskii
2021-09-08  7:02                               ` Michael Albinus
2021-09-08  3:17                     ` Barton, Mark
2021-09-08  5:09                       ` Augusto Stoffel
2021-09-08  7:50                     ` Lars Ingebrigtsen
2021-09-08 14:05                       ` Augusto Stoffel
2021-09-09 13:48                         ` Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).