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

Unpin nbconvert when upstream fix is released #471

Closed
desilinguist opened this issue Sep 29, 2020 — with Slack · 2 comments · Fixed by #484
Closed

Unpin nbconvert when upstream fix is released #471

desilinguist opened this issue Sep 29, 2020 — with Slack · 2 comments · Fixed by #484
Assignees

Comments

Copy link
Member

desilinguist commented Sep 29, 2020

Currently we have pinned nbconvert since v6.0 and above break custom templates. However, there's a fix probably on the way at some point. In the next release, we should remove the pin assuming the fix has been released.

@desilinguist desilinguist added the dependency label Sep 29, 2020 — with Slack
@desilinguist desilinguist self-assigned this Oct 2, 2020
@desilinguist
Copy link
Member Author

A potential fix was recently merged upstream but there has been no release yet. I was curious how we would incorporate this fix so I tried the following:

  • In my existing development conda environment, I ran conda remove nbconvert --force. I then installed the latest master from the nbconvert repo as follows: pip install pip install git+https://github.com/jupyter/nbconvert.git.

  • I edited the requirements file to remove the nbconvert pin and re-ran pip install -e .

  • I made the following changes to reporter.py.

             # set a high timeout for datasets with a large number of features
             report_config = Config({'ExecutePreprocessor': {'enabled': True,
                                                             'timeout': 3600},
    -                                'HTMLExporter': {'template_path': [template_path],
    -                                                 'template_file': 'report.tpl'}})
    +                                'HTMLExporter': {'template_name': 'classic',
    +                                                 'template_file': join(template_path, 'report.tpl')}})
  • I made the following changes to report.tpl:

    -{%- extends 'full.tpl' -%}
    +{%- extends 'index.html.j2' -%}

And with these changes, the report generation works fine and the report looks exactly as it does with nbconvert 5.6.1!

Assuming that things do not change drastically between now and whenever the next nbconvert release is, this should be a relatively easy fix.

@desilinguist
Copy link
Member Author

desilinguist commented Oct 28, 2020

Turns out it isn't that simple. The latest nbconvert is also broken on Windows for Python 3.8. The summary is that nbconvert uses nbclient which uses asyncio and that has an entirely different default implementation on Windows that doesn't include all the same functions! 🤦🏽 On top of that, since we are using nbconvert via its API and not the command line, we need to manually include the workaround in our code too. I will do this as part of this issue.

A fuller description of the issue can be found here and here.

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.

1 participant