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 multiline comments instruction for percent #403

Merged
merged 4 commits into from
Dec 24, 2019

Conversation

zhuoqiang
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Dec 20, 2019

Codecov Report

Merging #403 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #403      +/-   ##
==========================================
+ Coverage   99.05%   99.05%   +<.01%     
==========================================
  Files          75       75              
  Lines        7538     7545       +7     
==========================================
+ Hits         7467     7474       +7     
  Misses         71       71
Impacted Files Coverage Δ
tests/test_read_simple_percent.py 100% <100%> (ø) ⬆️
jupytext/cell_reader.py 96.94% <100%> (ø) ⬆️
tests/test_header.py 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cc0d83...4535f78. Read the comment docs.

@mwouts mwouts added this to the 1.3.1 milestone Dec 21, 2019
@mwouts
Copy link
Owner

mwouts commented Dec 23, 2019

Hello @zhuoqiang , thanks for spotting this.

I would say it is not required to double the triple quote, i.e. {"jupytext": {"cell_markers": "\"\"\""}} or c.ContentsManager.default_cell_markers = '"""' is enough, cf. also the tests below. Do you agree?

def test_multiline_comments_in_markdown_1():
text = """# %% [markdown]
'''
a
long
cell
'''
"""
nb = jupytext.reads(text, 'py')
assert len(nb.cells) == 1
assert nb.cells[0].cell_type == 'markdown'
assert nb.cells[0].source == "a\nlong\ncell"
py = jupytext.writes(nb, 'py')
compare(py, text)
def test_multiline_comments_in_markdown_2():
text = '''# %% [markdown]
"""
a
long
cell
"""
'''
nb = jupytext.reads(text, 'py')
assert len(nb.cells) == 1
assert nb.cells[0].cell_type == 'markdown'
assert nb.cells[0].source == "a\nlong\ncell"
py = jupytext.writes(nb, 'py')
compare(py, text)
def test_multiline_comments_format_option():
text = '''# %% [markdown]
"""
a
long
cell
"""
'''
nb = new_notebook(cells=[new_markdown_cell("a\nlong\ncell")],
metadata={'jupytext': {'cell_markers': '"""',
'notebook_metadata_filter': '-all'}})
py = jupytext.writes(nb, 'py:percent')
compare(py, text)
def test_multiline_comments_in_raw_cell():
text = '''# %% [raw]
"""
some
text
"""
'''
nb = jupytext.reads(text, 'py')
assert len(nb.cells) == 1
assert nb.cells[0].cell_type == 'raw'
assert nb.cells[0].source == "some\ntext"
py = jupytext.writes(nb, 'py')
compare(py, text)
def test_multiline_comments_in_markdown_cell_no_line_return():
text = '''# %% [markdown]
"""a
long
cell"""
'''
nb = jupytext.reads(text, 'py')
assert len(nb.cells) == 1
assert nb.cells[0].cell_type == 'markdown'
assert nb.cells[0].source == "a\nlong\ncell"
def test_multiline_comments_in_markdown_cell_is_robust_to_additional_cell_marker():
text = '''# %% [markdown]
"""
some text, and a fake cell marker
# %% [raw]
"""
'''
nb = jupytext.reads(text, 'py')
assert len(nb.cells) == 1
assert nb.cells[0].cell_type == 'markdown'
assert nb.cells[0].source == "some text, and a fake cell marker\n# %% [raw]"
py = jupytext.writes(nb, 'py')
compare(py, text)
def test_cell_markers_option_in_contents_manager(tmpdir):
tmp_ipynb = str(tmpdir.join('notebook.ipynb'))
tmp_py = str(tmpdir.join('notebook.py'))
cm = jupytext.TextFileContentsManager()
cm.root_dir = str(tmpdir)
nb = new_notebook(cells=[new_code_cell('1 + 1'), new_markdown_cell('a\nlong\ncell')],
metadata={'jupytext': {'formats': 'ipynb,py:percent',
'notebook_metadata_filter': '-all',
'cell_markers': "'''"}})
cm.save(model=dict(type='notebook', content=nb), path='notebook.ipynb')
assert os.path.isfile(tmp_ipynb)
assert os.path.isfile(tmp_py)
with open(tmp_py) as fp:
text = fp.read()
compare(text, """# %%
1 + 1
# %% [markdown]
'''
a
long
cell
'''
""")
nb2 = jupytext.read(tmp_py)
compare_notebooks(nb, nb2)
def test_default_cell_markers_in_contents_manager(tmpdir):
tmp_ipynb = str(tmpdir.join('notebook.ipynb'))
tmp_py = str(tmpdir.join('notebook.py'))
cm = jupytext.TextFileContentsManager()
cm.root_dir = str(tmpdir)
cm.default_cell_markers = "'''"
nb = new_notebook(cells=[new_code_cell('1 + 1'), new_markdown_cell('a\nlong\ncell')],
metadata={'jupytext': {'formats': 'ipynb,py:percent',
'notebook_metadata_filter': '-all'}})
cm.save(model=dict(type='notebook', content=nb), path='notebook.ipynb')
assert os.path.isfile(tmp_ipynb)
assert os.path.isfile(tmp_py)
with open(tmp_py) as fp:
text = fp.read()
compare(text, """# %%
1 + 1
# %% [markdown]
'''
a
long
cell
'''
""")
nb2 = jupytext.read(tmp_py)
compare_notebooks(nb, nb2)

@zhuoqiang
Copy link
Contributor Author

@mwouts the problem is, if configured using single "tripper quotes" like following python file:

# ---
# jupyter:
#   jupytext:
#     cell_markers: '"""'
#     formats: ipynb,.py:percent
#     text_representation:
#       extension: .py
#       format_name: percent
# ---

# %%
print("hello")

It could not be opened in jupyter with the following error:

Unreadable Notebook: /f.py ValueError('not enough values to unpack (expected 2, got 1)')

change config to double the "tripper quotes" cell_markers: '""","""' fixed this.

@mwouts
Copy link
Owner

mwouts commented Dec 23, 2019

the problem is, if configured using single "tripper quotes" like following python file (...)

Oh that's interesting! Probably a bug, then... I'll have a look.

@mwouts
Copy link
Owner

mwouts commented Dec 23, 2019

Now it should be ok with triple quotes x1 - I have added a test that reproduces the issue you quoted (Thanks!!). If you're ok with this I'll do the merge.

@zhuoqiang
Copy link
Contributor Author

Now it should be ok with triple quotes x1 - I have added a test that reproduces the issue you quoted (Thanks!!). If you're ok with this I'll do the merge.

please, thanks for the quick fix.

@mwouts mwouts merged commit 02d2f30 into mwouts:master Dec 24, 2019
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