Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The shell buffer and "elpy-shell-toggle-dedicated-shell" #1328

Closed
Nicholas-Autio-Mitchell opened this issue Mar 27, 2018 · 7 comments
Closed
Assignees
Milestone

Comments

@Nicholas-Autio-Mitchell
Copy link

Nicholas-Autio-Mitchell commented Mar 27, 2018

Thank you for this package, specifically for a recent enhancement in the latest update; namely the ability to set a buffer to run into a specific shell. Here is the related enhancement issue - [#1232] .

I am having a few problems with this feature. I can assign buffers their own shells and send code to them as one would expect - works great!

The scenario:

If I use the function elpy-shell-toggle-dedicated-shell and select a different shell, there is no error or warning, however things don't work as expected. If I send a line of code from the buffer to the newly assigned shell, the literal line of code does appear in the new shell, however it is actually executed in the original shell.

The iPython interpreter of the original shell does update (the In [n] is incremented to In [n+1], but there is otherwise no text shown), whereas the newly assigned shell just gets the raw text and the interpreter seems to hang, e.g.:

In [6]: import matplotlib.pyplot as plt
test_loss = -np.inf
BATCH_SIZE_TRAIN = 512
BATCH_SIZE_VAL = 256

... this only updates to have In [7] if I go into the shell and hit Enter.
The values e.g. test_loss = -np.inf are not defined in that shell, i.e. they didn't run.

If I assign the buffer back to the original shell, things carry on working as expected. this behaviour occurs if I use either of the new functions to swap the shell assignment, i.e. elpy-shell-toggle-dedicated-shell or elpy-shell-set-local-shell.

Could this be linked to the way the iPython interpreter works within Emacs?

My setup:

  • elpy 1.18 (both Emacs and Python versions)
  • emacs version: 25.3.2 on Linux
  • working in a virtual environment, using pyvenv-workon
    elpy-config output:

Elpy Configuration

Virtualenv........: sigra-prep (/home/nicholas/anaconda3/envs/env1)
RPC Python........: 3.5.2 (/home/nicholas/anaconda3/envs/env1/bin/python3)
Interactive Python: ipython (/home/nicholas/anaconda3/envs/env1/bin/ipython)
Emacs.............: 25.3.2
Elpy..............: 1.18.0
Jedi..............: 0.11.0 (0.11.1 available)
Rope..............: Not found (0.10.7 available)
Autopep8..........: 1.3.3 (1.3.4 available)
Yapf..............: Not found (0.21.0 available)
Syntax checker....: flake8 (/home/nicholas/anaconda3/envs/env1/bin/flake8)

@galaunay - as you implemented this, any ideas what I might be doing wrong?

I'm happy to provide any further information :)

@galaunay
Copy link
Collaborator

Thank you for this well detailed report !

I was able to reproduce the bug.
This is apparently due to the fact that python-mode offers a mechanism for dedicated shells,
but that elpy overwrites it.

Could you check that your problem is effectively linked to this ?
Just overwrite python-shell-get-process-name with the following function:

(defun python-shell-get-process-name (dedicated)
  "Calculate the appropriate process name for inferior Python process.
If DEDICATED is t returns a string with the form
`python-shell-buffer-name'[`buffer-name'] else returns the value
of `python-shell-buffer-name'."
    python-shell-buffer-name)

If it does fix the problem, I will try to think about a more elegant way of fixing this.

@jorgenschaefer jorgenschaefer added this to the v1.20 milestone Mar 31, 2018
@Nicholas-Autio-Mitchell
Copy link
Author

Apologies for the slow response.

I started out by trying to reproduce my reported bug, but it seems to be fixed - was your siggestion above already made changes within elpy in the latest update? I had updated all packages before starting, but hadn't redefined the function as you suggested.

The definition of python-shell-get-process-name is not yours from above, but:

(defun python-shell-get-process-name (dedicated)
  "Calculate the appropriate process name for inferior Python process.
If DEDICATED is t returns a string with the form
`python-shell-buffer-name'[`buffer-name'] else returns the value
of `python-shell-buffer-name'."
  (if dedicated
      (format "%s[%s]" python-shell-buffer-name (buffer-name))
    python-shell-buffer-name))

Do you know if this has been changed recently?
Can we close this issue?

@galaunay
Copy link
Collaborator

I still have the issue.

It's due to the fact that python.el always send the code to a dedicated shell if it exists.
(This happens in python-shell-get-buffer).

So if you try to send the code to the global shell while a dedicated shell exists, the code will always be sent to the dedicated shell.

Steps to reproduce:

  • Open a python file
  • Enable elpy with (elpy-mode)
  • Switch to a dedicated shell with (elpy-shell-toggle-dedicated-shell)
  • Send the code to the dedicated shell with (elpy-shell-send-buffer)
  • Switch back to a global shell with (elpy-shell-toggle-dedicated-shell)
  • Try to send the code to the global shell (elpy-shell-send-buffer): The code is sent to the dedicated shell instead of the global shell.

@nicholas-mitchell Do you still have the issue in this situation ?

Solution ?

First solution

It is possible to fix this by overwriting python-shell-get-buffer or python-shell-get-process-name.
But overwriting python.el functions is very likely to lead to bugs in the futur...

Second solution

Kill dedicated shells when switching to a global shell, to be sure to avoid the aforementioned situation.
This is no optimal: One may want to keep a dedicated and a global shell in parallel.

I will try to find a better solution, or implement the second one I thing.

galaunay added a commit to galaunay/elpy that referenced this issue Apr 20, 2018
The name of the dedicated python shells are now using the buffer name
without extension.
This avoid conflicts with the way python.el handles dedicated shells
(see jorgenschaefer#1328).
galaunay added a commit to galaunay/elpy that referenced this issue Apr 20, 2018
The name of the dedicated python shells are now using the buffer name
without extension.
This avoid conflicts with the way python.el handles dedicated shells
(see jorgenschaefer#1328).
@Nicholas-Autio-Mitchell
Copy link
Author

I just tried again and am able to reproduce the error. Not sure what I had done differently to report that things were working as desired! It was on a different computer, but with a cloned Emacs setup, both on Linux.

Just a quick FYI (in case it makes a difference somehow): I use the command elpy-shell-send-statement-and-step, and not elpy-shell-send-buffer as you listed.

Would it be possible to store a buffer-specific variable that just tracks the target shell for sending commands? Toggling using elpy-shell-toggle-dedicated-shell would then just set this variable as required. This is essentially your first solution, but without overwriting Python mode directly, just including a tiny handler in the middle?
... I admit I don't know the inner workings of either Elpy or Python modes themselves, so perhaps this isn't straightforward/possible?

Once last thing I will mention, I remember using this feature within Emacs using ESS mode macs Speaks Statistics, I believe - and for R instead of Python), which worked very well. It may be worth looking into their implementation for tips.

@galaunay
Copy link
Collaborator

Finally, I tried a third solution.
I just changed the convention for naming dedicated shells in PR #1353.

If it appears that it's not a viable solution, I will try to see if yours can be implemented.
Thanks for the feedback.

@Nicholas-Autio-Mitchell
Copy link
Author

Apologies for the slow testing on my part.

Everything now works just fine for me using your changes made in PR #1353 👍
From within a single script I can switch between the target python interpreter. Even better, it seems to be working well when using a different virtual environment for each of the interpreters!

Thanks again @galaunay

@galaunay
Copy link
Collaborator

Good news then !

Thank for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants