[PATCH] Fix ob-python.el initiate session error with py-shell

[PATCH] Fix ob-python.el initiate session error with py-shell

[stardiviner]

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


When I have a python src block like this:

#+begin_src python :session "test" :results output
print("hello, world")
#+end_src

It will report session error.

This minor patch should fixed ~py-shell~ initiate Python session issue.

-- 
[ stardiviner ]
       I try to make every word tell the meaning what I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
      

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-lisp-ob-python.el-org-babel-python-initiate-session-.patch --]
[-- Type: text/x-patch, Size: 1100 bytes --]

From fa39b7f3ad8aafe0fcebb27c5f695edf08820884 Mon Sep 17 00:00:00 2001
From: stardiviner <numbchild@gmail.com>
Date: Tue, 18 Feb 2020 20:52:32 +0800
Subject: [PATCH] * lisp/ob-python.el
 (org-babel-python-initiate-session-by-key):

fix ob-python.el initiate session issue.
---
 lisp/ob-python.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index de718b04b..f67acf2f9 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -207,8 +207,8 @@ (defun org-babel-python-initiate-session-by-key (&optional session)
 			     "^\\*\\([^*]+\\)\\*$" "\\1" python-buffer)
 			  (concat "Python-" (symbol-name session))))
 	       (py-which-bufname bufname))
-	  (py-shell)
-	  (setq python-buffer (org-babel-python-with-earmuffs bufname))))
+	  (setq python-buffer (org-babel-python-with-earmuffs bufname))
+	  (py-shell nil nil t org-babel-python-command python-buffer nil nil t nil)))
        (t
 	(error "No function available for running an inferior Python")))
       (setq org-babel-python-buffers
-- 
2.25.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 515 bytes --]

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Bastien]

Hi Stardiviner,

I add Jack to the loop, as he's deciding for these patches.

Jack: feel free to go ahead with committing, the release of 9.4
is not yet planned (I'll write to the list when it is.)

Thanks!

stardiviner <numbchild@gmail.com> writes:

> When I have a python src block like this:
>
> #+begin_src python :session "test" :results output
> print("hello, world")
> #+end_src
>
> It will report session error.
>
> This minor patch should fixed ~py-shell~ initiate Python session issue.

-- 
 Bastien

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Jack Kamm]

Hi stardiviner,

> This minor patch should fixed ~py-shell~ initiate Python session issue.

It looks like this patch only affects users of python-mode.el. Since I
don't use python-mode.el, I cannot test it. But the patch looks
fine. Please add a commit message and changelog entry, and I'll merge it
in.

Now, a few general thoughts on python-mode.el:

I'm a little surprised to see python-mode.el pop up, and that
we have code in ob-python to explicitly support it. I thought it was
obsolete ever since python.el was added to emacs 24. But, it looks like
it's still receiving commits in 2020, so I guess I was wrong.

In a sense, it doesn't seem right to have code in ob-python explicitly
supporting python-mode.el. While at one point it may have been a
contender with python.el as the standard Python mode, at this point it's
just another third-party package like elpy, jedi, etc, none of which get
explicit support.

I don't want to break anyone's setup without further investigation, so
I'll leave the python-mode.el support as is, for now. But a warning,
this may change in future if we ever refactor ob-python.

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Jack Kamm]

Hi stardiviner,

Sorry for the noise, I have some additional comments below:

When I run "make compile", I get the following message:

In org-babel-python-initiate-session-by-key:
ob-python.el:211:12:Warning: py-shell called with 9 arguments, but accepts
    only 0-1

I think this is because the "(declare-function py-shell)" no longer
matches the signature of the upstream py-shell function you're using.

Also, I now see that the commit does have a changelog entry, but not a
commit message. Unless it is standard to take the subject line as the
commit message? I am still a little new to merging patches from email,
and might not have all the proper tooling setup for it.

When you update the patch, please mention that this is specifically for
python-mode.el in the commit message and changelog entry.

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Bastien]

Hi Jack,

Jack Kamm <jackkamm@gmail.com> writes:

> Unless it is standard to take the subject line as the
> commit message?

Yes, it is - although in stardiviner's patch, the subject line should
not have the asterisk.

You can check for the propre format of a patch by doing

~$ git format-patch HEAD~10

and read the 10 latest patches (hopefully they are well formatted.)

I often do tiny edits on patches to fix problems, don't hesitate to
do so when it does not affect the author's intent.

Thanks,

-- 
 Bastien

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[numbchild]

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

Yes, Jack, as Bastien said, you can format my commit, because my home
network is broken, I'm using Mobile Phone's 4G network to get online. Can't
get update immediately.
And thanks for tips about `python-mode' is deprecated. I didn't know that.
I will migrate to `python.el'.

[stardiviner]           <Hack this world!>      GPG key ID: 47C32433
IRC(freeenode): stardiviner                     Twitter:  @numbchild
Key fingerprint = 9BAA 92BC CDDD B9EF 3B36  CB99 B8C4 B8E5 47C3 2433
Blog: http://stardiviner.github.io/


On Wed, Feb 19, 2020 at 1:18 AM Jack Kamm <jackkamm@gmail.com> wrote:

> Hi stardiviner,
>
> Sorry for the noise, I have some additional comments below:
>
> When I run "make compile", I get the following message:
>
> In org-babel-python-initiate-session-by-key:
> ob-python.el:211:12:Warning: py-shell called with 9 arguments, but accepts
>     only 0-1
>
> I think this is because the "(declare-function py-shell)" no longer
> matches the signature of the upstream py-shell function you're using.
>
> Also, I now see that the commit does have a changelog entry, but not a
> commit message. Unless it is standard to take the subject line as the
> commit message? I am still a little new to merging patches from email,
> and might not have all the proper tooling setup for it.
>
> When you update the patch, please mention that this is specifically for
> python-mode.el in the commit message and changelog entry.
>

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

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Jack Kamm]

Hi stardiviner,

"numbchild@gmail.com" <numbchild@gmail.com> writes:

> Yes, Jack, as Bastien said, you can format my commit, because my home
> network is broken, I'm using Mobile Phone's 4G network to get online. Can't
> get update immediately.

OK, I've edited the commit message of your patch, and I also fixed the
compilation warning by updating the signature of "(declare-function
py-shell)".

Commit has now been pushed to master :)

> And thanks for tips about `python-mode' is deprecated. I didn't know that.
> I will migrate to `python.el'.

Since python.el is built-in to Emacs, I think that things should
continue to work for python-mode.el users, even if we entirely switch
over to only using python.el.

The only thing I'm not sure about, is whether the equivalent of
"python-shell-send-region" in python-mode.el will work with shells
started by python.el. If not, this could probably be addressed with a
patch to python-mode.el.

You could test this by explicitly setting `org-babel-python-mode' to
`python' (it is probably set to `python-mode' on your system, which is
the default when python-mode.el is detected).

I'll add a TODO for myself to explicitly mark python-mode-related
variables as deprecated. I'm also planning a major update to the Worg
documentation of ob-python when 9.4 comes out, and will mention the
deprecation there as well.

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Bastien]

Hi Jack,

Jack Kamm <jackkamm@gmail.com> writes:

> I'll add a TODO for myself to explicitly mark python-mode-related
> variables as deprecated. I'm also planning a major update to the Worg
> documentation of ob-python when 9.4 comes out, and will mention the
> deprecation there as well.

Worg needs more love - thanks for taking care of this!

-- 
 Bastien

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[stardiviner]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256


Jack Kamm <jackkamm@gmail.com> writes:

> Hi stardiviner,
>
> "numbchild@gmail.com" <numbchild@gmail.com> writes:
>
>> Yes, Jack, as Bastien said, you can format my commit, because my home
>> network is broken, I'm using Mobile Phone's 4G network to get online. Can't
>> get update immediately.
>
> OK, I've edited the commit message of your patch, and I also fixed the
> compilation warning by updating the signature of "(declare-function
> py-shell)".
>
> Commit has now been pushed to master :)
>
>> And thanks for tips about `python-mode' is deprecated. I didn't know that.
>> I will migrate to `python.el'.
>
> Since python.el is built-in to Emacs, I think that things should
> continue to work for python-mode.el users, even if we entirely switch
> over to only using python.el.
>
> The only thing I'm not sure about, is whether the equivalent of
> "python-shell-send-region" in python-mode.el will work with shells
> started by python.el. If not, this could probably be addressed with a
> patch to python-mode.el.
>
> You could test this by explicitly setting `org-babel-python-mode' to
> `python' (it is probably set to `python-mode' on your system, which is
> the default when python-mode.el is detected).
>
> I'll add a TODO for myself to explicitly mark python-mode-related
> variables as deprecated. I'm also planning a major update to the Worg
> documentation of ob-python when 9.4 comes out, and will mention the
> deprecation there as well.

Thanks for your work! Jack

- -- 
[ stardiviner ]
       I try to make every word tell the meaning what I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
      
-----BEGIN PGP SIGNATURE-----

iQFIBAEBCAAyFiEE8J9lDX1nSBmJJZFAG13xyVromsMFAl5OHPIUHG51bWJjaGls
ZEBnbWFpbC5jb20ACgkQG13xyVromsMoIAgAupUPheHI9UoqcB7MIAH6qOZoZKfi
A7WurI++ioXEUN6rBlGkv/cNA2WsOm7TIjfLq1uXIEHvfgLY4nM+0YQJmPX1XzGr
ej+xNO+9sjwrXA83XELM5DcGhat2C3PBE0ANqZIeZxnE1roUOcYVCdIdrPsvrpWC
/nSHIEB3fg19lq+QGwr17qXqLsNy/BjytixWclHQz9bmA5Cs9p9jGPDIamrEK+4w
7/1QMXg1B3GUPWrRLUfeMV3hye+btpPZOqVpk1OwVg0AJpp4brI/OvkzOrfDMhA4
fxjB/JE2Ll4hvN+MSJ0wGohMUIvClm9Li6jWsKmC1ZHtAdzn5lkKQ2nspA==
=VCww
-----END PGP SIGNATURE-----

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Andreas Röhler]

,

On 20.02.20 06:45, stardiviner wrote:

Hi,
python-mode.el developer here.
>> Since python.el is built-in to Emacs, I think that things should
>> continue to work for python-mode.el users, even if we entirely switch
>> over to only using python.el.

Thanks supporting python-mode.el. Both python-modes should not conflict 
- except for the key-setting, which is taken by the last one loaded:

py-mode-map didn't work for some reasons, so both provide now 
python-mode-map.

>>
>> The only thing I'm not sure about, is whether the equivalent of
>> "python-shell-send-region" in python-mode.el will work with shells
>> started by python.el. If not, this could probably be addressed with a
>> patch to python-mode.el.

It should work. Will check it. Please feel free to send a bug-report 
resp. feature request to

https://gitlab.com/python-mode-devs/python-mode/issues

in case of python-mode.el related stuff.


>> You could test this by explicitly setting `org-babel-python-mode' to
>> `python' (it is probably set to `python-mode' on your system, which is
>> the default when python-mode.el is detected).
>>
>> I'll add a TODO for myself to explicitly mark python-mode-related
>> variables as deprecated. I'm also planning a major update to the Worg
>> documentation of ob-python when 9.4 comes out, and will mention the
>> deprecation there as well.
> Thanks for your work! Jack
>
> - -- 
> [ stardiviner ]

BTW there are some reasons, why python-mode.el still exists: no known 
indentation bugs, finer navigation commands etc. Maybe have a look at 
open python.el bugs and check them against python-mode.el

Best,

Andreas

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Bastien]

Hi all,

in future releases of Org, we will move toward stricter rules on what
to accept as ob-* libraries and what modes these libraries should rely
upon.

This was briefly mentioned in this email:
https://lists.gnu.org/archive/html/emacs-orgmode/2020-02/msg00714.html

For example, we can have a rule that we don't include ob-* libraries
for languages that are not somehow known by Emacs.  It would mean that
e.g. ob-clojure.el would have to live outside Org's core.

Also, for included ob-*, the idea would be to use the mode that are
bundled with Emacs core.  For Python, it would mean that ob-python.el
should support python.el, not python-mode.el.

This is not to say that python.el is better than python-mode.el: I
have no opinion on this and I'm confident both meet different users.

But that's something to consider.

We can always have another ob-python-mode.el file for users who use
python-mode.el: even the names would make it clear that Org babel uses
one mode and not the other.

I suggest we have the discussion on such rules after Org 9.4, but in
the meantime, this is something you may consider when fixing hacking
on ob-python.el.

Best,

-- 
 Bastien

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Jack Kamm]

Hi,

Bastien <bzg@gnu.org> writes:

> Also, for included ob-*, the idea would be to use the mode that are
> bundled with Emacs core.  For Python, it would mean that ob-python.el
> should support python.el, not python-mode.el.

I agree that ob-python.el should only rely on functionality from
python.el, and shouldn't need to know about other packages. This will
reduce the maintenance burden for ourselves.

At the same time, I would like the user to use whatever Python-related
modes they like when editing the org Python source buffer, such as
python-mode.el, elpy, etc., without issue. That doesn't mean that we'll
put workarounds and references to external packages in ob-python.el, but
rather that we'd keep ob-python simple and easy for external packages to
interop with.

I think this is more or less the case today, and we're on the same page
here. Just wanted to make this goal explicit.

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[Bastien]

Hi Jack,

Jack Kamm <jackkamm@gmail.com> writes:

> I think this is more or less the case today, and we're on the same page
> here. Just wanted to make this goal explicit.

OK, thanks for the precision!

-- 
 Bastien

Re: [PATCH] Fix ob-python.el initiate session error with py-shell

[stardiviner]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256


Bastien <bzg@gnu.org> writes:

> Hi all,
>
> in future releases of Org, we will move toward stricter rules on what
> to accept as ob-* libraries and what modes these libraries should rely
> upon.
>
> This was briefly mentioned in this email:
> https://lists.gnu.org/archive/html/emacs-orgmode/2020-02/msg00714.html
>
> For example, we can have a rule that we don't include ob-* libraries
> for languages that are not somehow known by Emacs.  It would mean that
> e.g. ob-clojure.el would have to live outside Org's core.

This is a bad news for me. I understand your considering. Well, I hope there is
a fine handling of this ob-* libraries which will be outside of Org Mode core.

Best Regards.

- -- 
[ stardiviner ]
       I try to make every word tell the meaning what I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
      
-----BEGIN PGP SIGNATURE-----

iQFIBAEBCAAyFiEE8J9lDX1nSBmJJZFAG13xyVromsMFAl5ROosUHG51bWJjaGls
ZEBnbWFpbC5jb20ACgkQG13xyVromsPf4ggAqEfRGuZoVOw83G6KvVCVTCEFgZ7b
/mz+quLLYzvb43BU8jShIDC+vYYz1rYpyjg7sL0WJddFdko7MGD9NHd4/ZfmbYOg
s+jaIS2isrXtTMZSFRfKSZvqyNuTQQcQhLSQOGSYy3UePKcl1eABCJnS/eSrn+jp
adQP7ieJoboKrmORsAGIvRPBgPPpML3u0P9lAJbSxfMSCOaGM5/MNS0uRNrB2cjT
dk82ohLle2D0szGxMusBkpRsNu2HyRvmL2kVKyP99BzK949FGFldzO4HsW7Pb5lo
3lae5pcJLa+aYXtbFRMf8GXTKCmdGY5hf9hKaVj85Z4++9C5xpeZn4ca5Q==
=zVp9
-----END PGP SIGNATURE-----