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

Comprehensive edit doc files for grammar and understanding #1389

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

willingc
Copy link
Member

@willingc willingc commented Sep 13, 2020

In trying to add (Do subclasses need to call super().preprocess_cell() now?), I wound up cleaning up other pages.

@willingc willingc marked this pull request as ready for review September 13, 2020 21:42
@willingc willingc requested a review from MSeal September 13, 2020 21:42
Automating the data analysis in projects involving more than one notebook
is also possible.

Notebook execution can be done either from the command line or programmatically
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add a note that nbclient is the natural successor for programmatic execution of notebooks, but that the interface remains here in nbconvert for extension and mixin of the preprocessor concept, and that papermill has a more extensive and robust command line interface than nbconvert for stand alone notebook execution. Many people using nbconvert execute don't know about those other options or when one is better to use.

@@ -349,7 +349,7 @@
"source": [
"There are an endless number of transformations that you may want to apply to a notebook. In particularly complicated cases, you may want to actually create your own `Preprocessor`. Above, when we customized the list of preprocessors accepted by the `HTMLExporter`, we passed in a string -- this can be any valid module name. So, if you create your own preprocessor, you can include it in that same list and it will be used by the exporter.\n",
"\n",
"To create your own preprocessor, you will need to subclass from `nbconvert.preprocessors.Preprocessor` and overwrite either the `preprocess` and/or `preprocess_cell` methods."
"To create your own preprocessor, you will need to subclass from `nbconvert.preprocessors.Preprocessor` and overwrite either the `preprocess` and/or `preprocess_cell` methods. As of version 6.0, subclasses likely will need to call `super().preprocess_cell()` or something functionally equivalent to `NotebookClient.async_execute_cell`. In general, `super().preprocess_cell()` is best to minimize the complexity and subtlety of the multi-inheritance and overrides at play."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The super().preprocess_cell() or something functionally equivalent to 'NotebookClient.async_execute_cell'section only applies to ExecutePreprocessor. The more general case is that if the parent class does not raiseNotImplementedin it's definition then it's expected to havesuper().preprocess_cell(...)` called. As with all inheritance if you understand what the parent is doing and wish you can totally replace the calls and not call super.

format given by the ``FORMAT`` string.

Default output format - HTML
----------------------------
The default output format is HTML, for which the ``--to`` argument may be
nbconvert's default output format is HTML, and the ``--to`` argument may be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh this changed actually -- if you do not specify a --to you will get ValueError: Please specify an output format with '--to <format>'. I didn't realize we missed the doc update for that :/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That probably means we need to find all the jupyter nbconvert and change them to jupyter nbconvert --to=html unless they have a to argument. I can take that after this merges if you like.

@SylvainCorlay
Copy link
Member

Thanks @willingc ! Many thanks for all the changes.

@SylvainCorlay
Copy link
Member

Hey @willingc are you ok with the proposed changes by @MSeal?

If you are too busy, I am happy to integrate them and merge.

@willingc
Copy link
Member Author

@SylvainCorlay The changes are fine by me. Feel free to integrate if you have time. Otherwise, I will loop back later in the day.

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.

None yet

3 participants