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

With nbconvert 5.3 you cannot use the ExecutePreprocessor and filter outputs at the same time #663

Open
mpacer opened this issue Sep 2, 2017 · 7 comments

Comments

@mpacer
Copy link
Member

mpacer commented Sep 2, 2017

TagRemovePreprocessor currently handles removing cells, inputs, all outputs and individual outputs. It runs before the ExecutePreprocessor runs.

As a result, once ExecutePreprocessor runs, it recreates the outputs that should have been filtered out by TagRemovePreprocessor.

The simplest solution is to rerun TagRemovePreprocessor but that seems wasteful.

The other solution would be to create a subclass of TagRemovePreprocessor called something like OutputTagRemovePreprocessor that inherits the traitlet values, but overwrites the preprocess_cell method to only filter outputs (adhering to both remove_all_outputs_tags and remove_single_output_tags). Then invoke this new preprocessor after all other preprocessors have run. This would make sense to appear in the base Exporter class.

@mpacer mpacer added the good first issue great for new contributors label Sep 2, 2017
@mpacer
Copy link
Member Author

mpacer commented Sep 2, 2017

@pxhanus Are you interested in tackling this one?

@jzf2101
Copy link

jzf2101 commented Sep 28, 2017

@mpacer can we add this one to hacktoberfest?

@pxhanus
Copy link
Contributor

pxhanus commented Sep 28, 2017

@mpacer Yes, I've got this! I'll let you know if I have any questions.

@mpacer
Copy link
Member Author

mpacer commented Oct 4, 2017

@pxhanus, why don't you start by just making a PR that readds the TagRemovePreprocessor?

@jzf2101
Copy link

jzf2101 commented Nov 20, 2017

@pxhanus are you still interested, I think @fenwickipedia might consider it

@pxhanus
Copy link
Contributor

pxhanus commented Dec 8, 2017

@jzf2101 @fenwickipedia I am still working on this. Thanks for checking!

@fcollonval
Copy link
Contributor

There is a temporary fix proposed in #1300 (comment)

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

5 participants