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

Fix error when pickling HTMLExporter #1435

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kycutler
Copy link
Contributor

@kycutler kycutler commented Oct 2, 2020

Argh. Turns out the HTML exporter had a similar issue to the TemplateExporter (in #1398) which causes it to fail to pickle. This PR fixes the HTML exporter as well.

I checked the other exporters manually and none appeared to have nested functions, but I didn't write tests for all of them.

@MSeal
Copy link
Contributor

MSeal commented Oct 2, 2020

Thanks for helping dig into those pickling issues more. It does look like it's still not hope with pickling those functions in the exporter though based on that stack trace

@kycutler
Copy link
Contributor Author

kycutler commented Oct 3, 2020

Yeah my bad, I missed the failed test in Python3.7 on my local. I fixed those errors (including in Python3.6) but now it looks like the cannot pickle 'weakref' object errors may be caused by something in Jinja... I'm wondering if there is a way to refactor these three resources to avoid this, but I'm afraid I'm out of my depth there.

For my specific use case, I only need the output, which is feasible with these current changes:

import nbconvert, nbformat
from concurrent.futures import ProcessPoolExecutor

def do_export(exporter, notebook):
    (output, resources) = exporter.from_notebook_node(notebook)
    return output

if __name__ == "__main"":
  executor = ProcessPoolExecutor()
  exporter = nbconvert.exporters.HTMLExporter()
  nb = nbformat.read('example.ipynb', as_version=4)

  output = executor.submit(do_export, exporter, nb).result() # succeeds

Can this be considered "good enough"? Or do you have any ideas about fixing the resource pickling?

@MSeal
Copy link
Contributor

MSeal commented Oct 6, 2020

Try moving those resource functions outside of the class entirely and accept an env argument. Then wrap the resource assignments with a lambda / partial function that supplies the self.environment field as the first argument. I think that would avoid having the full self weak reference present but have the environment and name still be dynamic.

@kycutler
Copy link
Contributor Author

kycutler commented Oct 7, 2020

Hmm, that didn't quite work. It looks like there are references to the exporter within the environment -- for example environment.filters["filter_data_type"].parent, or any of the other filters that inherit from LoggingConfigurable and keep a reference to the parent.

@jasongrout jasongrout marked this pull request as draft September 13, 2021 23:58
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 this pull request may close these issues.

2 participants