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

Nbconvert's shebang ignores the version of the kernel in every notebook that is converted to Python #1390

Closed
fulopkovacs opened this issue Sep 13, 2020 · 4 comments · Fixed by #1391

Comments

@fulopkovacs
Copy link

This is the shebang that nbconvert prepends to the beginning of every notebook that it converts to python:

#!/usr/bin/env python

Since the shebang is always the same, the specific version of the Python kernel in the notebook is always ignored, which is unfortunate, since one could have multiple kernels installed at the same time (for example I have Python3.6, Python3.7 and Python3.8 notebooks).

 These steps might only apply to Unix systems.

  1. Create multiple notebooks with different versions of Python kernels. They should have one cell, containing the following lines:
import sys
print(sys.version)
  1. Run the cells in the notebooks. You should see the correct kernel versions in the outputs.
  2. Now convert the notebooks to python with nbconvert:
$ jupyter nbconvert --to python your-notebook.ipynb
  1. Observe the shebangs at the beginning of the files. They should the same (#!/usr/bin/env python).
  2. Now make the scripts executable, and run them from the command line. In the outputs you should see the same version of Python, no matter which version your notebooks had
$ chmod +x your-notebook.py
$ ./your-notebook.py

I uploaded a Python notebook, that has two cells:

  • Cell 1.: prints the kernel version that the notebook uses (should be Python3.6)
  • Cell 2.: prints the Python version that is defined by the shebang when the notebook is converted to python. This value depends on whatever the first binary named "python" is in you PATH (on Unix systems, I don't know about Windows).
    If the outputs are different, the problem is obvious. If they are not, then check out the five steps I mentioned below, which provide you a much better way to reproduce the issue.
    python-3.6.txt

Nbconvert version: 5.6.1

@MSeal
Copy link
Contributor

MSeal commented Sep 14, 2020

Hmm while that is correct that it doesn't use kernel information to populate the shebang, not all machines have a python3 or python3.8 installed, so you might end up with a shebang that couldn't be respected. Also not all kernels specify their python version so it's sometimes impossible to respect this:

{
 "argv": [
  "/home/mseal/.py3local/bin/python",
  "-m",
  "ipykernel_launcher",
  "-f",
  "{connection_file}"
 ],
 "display_name": "Python 3",
 "language": "python"
}

as an example kernel spec file on my machine.

One could launch the kernel, read the sys.version, and use that to fix the latter issue. Though I'd be hesitant to add that to the preprocessor as it's a LOT slower and process heavy to do that than the current preprocessor.

Would adding an argument option to the preprocessor to allow you to set the shebang python version meet needs?

@fulopkovacs
Copy link
Author

fulopkovacs commented Sep 14, 2020

Hmm while that is correct that it doesn't use kernel information to populate the shebang, not all machines have a python3 or python3.8 installed, so you might end up with a shebang that couldn't be respected. Also not all kernels specify their python version [...].

These are valid points, I absolutely agree with them.

Would adding an argument option to the preprocessor to allow you to set the shebang python version meet needs?

Well, it doesn't really seem to address my main issue with the shebang: the fact that in its current form it might produce unexpected results, which falls into the "bug" category for me.

If I use a Python3.8 kernel in a notebook, I think it's reasonable for me to expect the converted script either

  • choosing the same Python version as interpreter,
  • or not specifying any Python interpreter at all.

To be honest I'd prefer the second option, simply because I don't think that it's nbconvert's job to produce executable Python scripts. In my opinion it's enough if nbconvert converts the Python notebooks to Python scripts and then it's up to the user to add the shebangs and change the permissions.

I personally would be satisfied (to different extents though) with the following solutions for this issue:

  1. "Revoking" the shebang-feature
  2. Leaving it as it is due to compatibility issues/popular demand. I understand that some people might rely on this feature, and don't want to mess with them. It would be still great to add a few lines about this problem to the documentation, just in case someone will need it. (I created this issue partly because of this reason: it's not a problem for me, but someone eventually might come here through a web search and understand why their script weren't finding the dependencies they installed for the notebook's kernel version, but not the python kernel specified in the PATH, etc...)

Another argument for revoking the feature would be that that even though the shebang-idea was was a proposal from the community (and eventually a PR): #690, I don't really see many people asking for it (in that specific issue, or in any other one).

An argument for leaving this situation as it is would be that my concerns had been (in a way) already addressed in the discussion of the PR, but they got brushed off with the following argument:

I'm inclined to just let it be python - even if that won't necessarily work all the time, I think it represents the intent of converting to a .py script. Even run in IPython, you can't guarantee that code from a notebook will work - it might depend on displaying rich output, for instance. (source)

While I do not agree with this argument, I can accept the reasoning, that the options were already considered, and a decision was already made by the project.

(By the way, thank you for the quick and helpful response! ☺️ )

@MSeal
Copy link
Contributor

MSeal commented Sep 14, 2020

Thanks for clearly documenting the decisions made in the past. I have a similar PoV as you do on your response for how I would approach it given a fresh project without a prior opinion. However I have found changing anything in this project breaks someone's workflow as there's a lot of subtle implicit edges exposed, and plus prior authors seemed to have a strong opinion on the setup. So I am inclined towards option 2 to avoid more pain unless other maintainers think we should go with something else and pipe up. For now I'll add documentation about the current state of things.

@fulopkovacs
Copy link
Author

👍 Option 2 with documentation seems to be a good solution considering all the points you mentioned.

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

Successfully merging a pull request may close this issue.

2 participants