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 installed from conda or pip fails because missing jupyter_client #556

Closed
damianavila opened this issue Mar 26, 2017 · 8 comments · Fixed by #559
Closed

nbconvert installed from conda or pip fails because missing jupyter_client #556

damianavila opened this issue Mar 26, 2017 · 8 comments · Fixed by #559
Milestone

Comments

@damianavila
Copy link
Member

Either if I install it using conda or pip I see this error:

Traceback (most recent call last):
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/bin/jupyter-nbconvert", line 11, in <module>
    sys.exit(main())
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/jupyter_core/application.py", line 267, in launch_instance
    return super(JupyterApp, cls).launch_instance(argv=argv, **kwargs)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/traitlets/config/application.py", line 658, in launch_instance
    app.start()
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/nbconvert/nbconvertapp.py", line 305, in start
    self.convert_notebooks()
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/nbconvert/nbconvertapp.py", line 463, in convert_notebooks
    self.exporter = cls(config=self.config)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/nbconvert/exporters/templateexporter.py", line 199, in __init__
    super(TemplateExporter, self).__init__(config=config, **kw)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/nbconvert/exporters/exporter.py", line 102, in __init__
    self._init_preprocessors()
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/nbconvert/exporters/exporter.py", line 252, in _init_preprocessors
    self.register_preprocessor(preprocessor)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/nbconvert/exporters/exporter.py", line 218, in register_preprocessor
    return self.register_preprocessor(preprocessor_cls, enabled)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/nbconvert/exporters/exporter.py", line 231, in register_preprocessor
    self.register_preprocessor(preprocessor(parent=self), enabled)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/traitlets/traitlets.py", line 958, in __new__
    inst.setup_instance(*args, **kwargs)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/traitlets/traitlets.py", line 986, in setup_instance
    super(HasTraits, self).setup_instance(*args, **kwargs)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/traitlets/traitlets.py", line 977, in setup_instance
    value.instance_init(self)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/traitlets/traitlets.py", line 1600, in instance_init
    self._resolve_classes()
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/traitlets/traitlets.py", line 1605, in _resolve_classes
    self.klass = self._resolve_string(self.klass)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/traitlets/traitlets.py", line 1507, in _resolve_string
    return import_item(string)
  File "/home/damian/miniconda3/envs/test_pip_nbconvert/lib/python3.6/site-packages/traitlets/utils/importstring.py", line 34, in import_item
    module = __import__(package, fromlist=[obj])
ModuleNotFoundError: No module named 'jupyter_client'

My best guess... we are adding the execute preprocessor in the preprocessor default list and when it is being registered, it fails: https://github.com/jupyter/nbconvert/blob/5.1.1/nbconvert/exporters/exporter.py#L72

I think this was missed because if you are developer, you installed all the dependencies... and if you are an user, you probably installed it in the same environment with the notebook, so you are covered.

Or I have something wrong in my environments and I have to start from scratch 😉

Proposal... if we decide to ship the execute preprocessor as a default one (which I believe is true) then let's just add the jupyter_client as a dependency.

Let me know if you agree and I will push a PR adding the dependency.

ping @mpacer @takluyver @Carreau @minrk which has been involved in previous discussion around jupyter_client here...

@mpacer
Copy link
Member

mpacer commented Mar 26, 2017

I'd been in favour of it before, I still am.

I forget exactly what the argument is for why execute should be thought to be an edge case feature.

I think there might be a reason beyond that. Along the lines of: "if you want to do this, you can probably figure out how to do it (if you are capable enough to be trusted to do this safely)." And I'm not sure I agree with that but execute is one of the more dangerous features that we ship so if we're ever going to take that attitude, it would seem that “getting the execute preprocesser to function” would be an ideal case.

So I'm conflicted but likely to be easily swayed in either direction based of further discussion. I imagine the topic will be about our role in enabling people with great power. But with great power comes the responsibility to not accidentally “shoot yourself in the foot”. The danger presented by the execute preprocessor is that it makes it easy to execute arbitrary code from someone else. Someone more novice might trust a file that in reality is untrustworthy

@mpacer mpacer closed this as completed Mar 26, 2017
@mpacer mpacer reopened this Mar 26, 2017
@mpacer
Copy link
Member

mpacer commented Mar 26, 2017

Oops hit close and comment by mistake.

@takluyver
Copy link
Member

Possibly the reason it's an 'extra' dependency is that it introduces a dependency on pyzmq, which may be harder to install in some situations, as it has compiled pieces. Now that we have wheels, however, it mostly doesn't make a big difference.

@damianavila
Copy link
Member Author

The big problem I see here is that I see the error even when I do not want to use the execute feature!
I am just doing jupyter nbconvert foo.ipynb
I am just trying to convert to html without any execute thing at all and, still, I see the error.
For me this is a broken user experience, regardless of the valid points that @mpacer raised.

@takluyver
Copy link
Member

Yep, that's definitely a bug. If jupyter_client remains an optional dependency, we should avoid importing it when it's not needed.

@minrk
Copy link
Member

minrk commented Mar 27, 2017

There probably isn't a good enough reason to exclude jupyter_client in the conda package, since we know it will always be easily installable in that context. There are still plenty of pip installations where pulling in things like zmq would be nice to avoid, though.

Even if it is made a strict dependency, the delayed import is still the right thing to do for startup time reasons.

@damianavila
Copy link
Member Author

damianavila commented Mar 28, 2017

There probably isn't a good enough reason to exclude jupyter_client in the conda package, since we know it will always be easily installable in that context.

Yep, I will ping the anaconda people so it gets added into the default channel recipe as well

Thank you all for the discussion!

@damianavila
Copy link
Member Author

Yep, I will ping the anaconda people so it gets added into the default channel recipe as well

Just to let you know, this was added in the package coming from default as well.

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

Successfully merging a pull request may close this issue.

4 participants