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

problem with python3 str #27

Open
tdaff opened this issue Aug 24, 2018 · 3 comments
Open

problem with python3 str #27

tdaff opened this issue Aug 24, 2018 · 3 comments

Comments

@tdaff
Copy link
Owner

tdaff commented Aug 24, 2018

Original report by Jean-Marc Andreoli (Bitbucket: jmandreoli, GitHub: jmandreoli).


The following lines (600-602) in kernel.py, which decode self.host, cause an error with slurm (and probably some other launchers), as the decoding happens too late.

#!python

# zmq needs str in Python 3, but pexpect gives bytes
if hasattr(self.host, 'decode'):
  self.host = self.host.decode('utf-8')

And furthermore, there is no need to do the decoding again and again each time tunnel_cmd is invoked.

My fix: Move them just after launch_* but before start_kernel at line 240

@tdaff
Copy link
Owner Author

tdaff commented Oct 8, 2018

Original comment by Matthias Bussonnier (Bitbucket: Carreau, GitHub: Carreau).


It's actually assign in a ton of places. I have a patch making it working on slurm, and force the assigment to always be string using Traitlets. I'm hitting other issues on slurm (mostly expect('exit') to block), but I should be able to send a patch soon.

@tdaff
Copy link
Owner Author

tdaff commented Oct 9, 2018

Original comment by Jean-Marc Andreoli (Bitbucket: jmandreoli, GitHub: jmandreoli).


Just FYI, I implemented the simple fix I mention in my comment (move the lines of code) and had no problem with slurm since then. remote_ikernel is a cool package, it would be nice if it could natively be integrated in jupyter, where the method to select a kernel could be split into the choice of kernel itself and the choice of launcher, instead of the current single scroll down menu combining both. Do you know if there are any plans along that line ? I am not familiar with the jupyter development process, but I for one would definitely support that feature.

@tdaff
Copy link
Owner Author

tdaff commented Oct 9, 2018

Original comment by Matthias Bussonnier (Bitbucket: Carreau, GitHub: Carreau).


You could definitely have a kernel manager that does it, but one of the problem is interoperability.
You need many pieces that are "Jupyter aware" to reimplement it instead if you "bake" that into jupyter.
One of the advantages here is that it will work with classic jupyter, nteract, papermill, hydrogen, .... etc So there is a strong incentive to keep pieces separated.

I've send https://bitbucket.org/tdaff/remote_ikernel/pull-requests/5/ensure-selfhost-is-always-unicode/diff that should fix that more globally across launchers.

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

1 participant