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.

Sign up for GitHub

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

Don't remove empty cells by default #784

Merged
merged 1 commit into from
Aug 29, 2018
Merged

Don't remove empty cells by default #784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions nbconvert/preprocessors/regexremove.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,14 @@ class RegexRemovePreprocessor(Preprocessor):
of unicode strings. If the contents match any of the patterns, the cell
is removed from the notebook.

By default, `patterns = [r'\Z']` which matches the empty string such that
strictly empty cells are removed. To modify the list of matched patterns,
To modify the list of matched patterns,
modify the patterns traitlet. For example, execute the following command
to convert a notebook to html and remove cells containing only whitespace:
to convert a notebook to html and remove cells containing only whitespace::
Copy link
Contributor

Choose a reason for hiding this comment

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

Small typo of an extra : here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually reST syntax, see http://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks

Without that, the formatting doesn't work, see the currently broken docs:
https://nbconvert.readthedocs.io/en/5.3.1/api/preprocessors.html#nbconvert.preprocessors.RegexRemovePreprocessor


> jupyter nbconvert --RegexRemovePreprocessor.enabled=True \
Copy link
Contributor

Choose a reason for hiding this comment

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

The default value for the enabled flag appears to be False. We should either override the default value for the RegexRemovePreprocessor or keep the --RegexRemovePreprocessor.enabled=True flag. Or is a preprocessor enabled automatically if one of its attributes (patterns) is set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tillahoffmann Thanks for the review!

I'm not quite sure, but I think the preprocessor is enabled by default because of this:

nbconvert/nbconvert/exporters/templateexporter.py

Lines 126 to 135 in 9f5c402

@property
def default_config(self):
c = Config({
'RegexRemovePreprocessor': {
'enabled': True
},
'TagRemovePreprocessor': {
'enabled': True
}
})

I don't care if it is enabled or not, as long as it doesn't remove stuff by default.

--RegexRemovePreprocessor.patterns="['\\s*\\Z']" mynotebook.ipynb
jupyter nbconvert --RegexRemovePreprocessor.patterns="['\\s*\\Z']" mynotebook.ipynb

The first command line argument enables the preprocessor and the second
sets the list of patterns to '\\s*\\Z' which matches an arbitrary number
The command line argument
sets the list of patterns to ``'\\s*\\Z'`` which matches an arbitrary number
Copy link
Contributor

Choose a reason for hiding this comment

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

Are single back ticks sufficient here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried it, and apparently not.

Sphinx allows to customize the formatting of single backticks, but it is apparently not configured for typewriter text (and I also don't think it would be a good idea to do so).

of whitespace characters followed by the end of the string.

See https://regex101.com/ for an interactive guide to regular expressions
Expand All @@ -38,7 +36,7 @@ class RegexRemovePreprocessor(Preprocessor):
documentation in python.
"""

patterns = List(Unicode(), default_value=[r'\Z']).tag(config=True)
patterns = List(Unicode(), default_value=[]).tag(config=True)

def check_conditions(self, cell):
"""
Expand Down
4 changes: 2 additions & 2 deletions nbconvert/preprocessors/tests/test_regexremove.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ def test_output(self):
'disallow_tab_newline': [r'\t\Z', r'\n\Z']
}
expected_cell_count = {
'default': 5, # only strictly empty cells
'default': 6, # nothing is removed
'disallow_whitespace': 2, # all "empty" cells are removed
'disallow_tab_newline': 3, # all "empty" cells but the single space
'disallow_tab_newline': 4, # cells with tab and newline are removed
'none': 6,
}
for method in ['default', 'disallow_whitespace', 'disallow_tab_newline', 'none']:
Expand Down