Support exporting markdown cells to notebook file #1498

kylebarron · 2018-12-25T01:32:11Z

This adds support for Markdown cells in the export-notebook command (closes #1296).

This requires the substring md or markdown to be present on the cell marker line, i.e. any of the following would work correctly:
```
 # %% md
 # %% markdown
 # %% [md]
 # %% [markdown]
 # In[1] md
 # In[1] markdown
 # In[1] [md]
 # In[1] [markdown]
 # <codecell> md
 # <codecell> markdown
 # <codecell> [md]
 # <codecell> [markdown]
```
Right now, # <md> and # <markdown> are not supported. It wouldn't be hard to support them; I'd just have to change this regex list:

hydrogen/lib/code-manager.js

Line 138 in ea00043

const regexString = `${escapedCommentStartString}(%%| %%| <codecell>| In\[[0-9 ]*\]:?)`;
If there's a comment symbol as the first characters on a line, it and any left whitespace will be removed.
There's always a cell created before the first # %% marker, even if it's on the first line. To me that's a bug, but that should be solved in another PR.

Any comments/code improvement suggestions are welcome. Hopefully this JS is a little more idiomatic than my last PR.

# %%
print('hello world')

# %% markdown
# # Headline1
# - bullet point
# **bold text**

# %%
print('bye world')

In Jupyter Notebook:

cc: @mwouts, @Madhu94

kylebarron · 2018-12-26T19:29:54Z

Should I make md and markdown case insensitive, so that the following would also be caught?

# %% Md
# %% Markdown

mwouts · 2018-12-26T20:52:07Z

Hello @kylebarron , thanks for the developement, this feature was wanted for a long time!

Is it correct that %% [markdown] cells from jupytext will also be recognized as markdown? Like in this example? As a quick reminder: we have choosen square brackets and not just the work markdown in order to support cell titles from Spyder, cf this thread.

kylebarron · 2018-12-26T20:59:06Z

# %% [markdown] would be supported fine. As long as the substring markdown (or md) exists somewhere after # %%, it'll be caught. I think that's better than us imposing another format.

I'm also working on a PR to import Notebook files into Hydrogen, and hopefully it'll be possible to import, modify, and export without any data loss. I'd love to explore a run all cells and export notebook command that adds cell output to the created notebook file.

mwouts · 2018-12-26T21:26:37Z

# %% [markdown] would be supported fine. As long as the substring markdown (or md) exists somewhere after # %%, it'll be caught. I think that's better than us imposing another format.

Very nice!

I'm also working on a PR to import Notebook files into Hydrogen, and hopefully it'll be possible to import, modify, and export without any data loss. I'd love to explore a run all cells and export notebook command that adds cell output to the created notebook file.

That sounds great! That will make Hydrogen the first text editor where users can act on the ipynb file without having to run the notebook in full a second time.

kylebarron · 2018-12-27T01:17:58Z

That sounds great! That will make Hydrogen the first text editor where users can act on the ipynb file without having to run the notebook in full a second time.

Well, not necessarily, at least not at first. I'm working on a simple PR to import just the source text. That's quite straightforward. In the short run, no outputs will be saved in import or export.

I don't know enough about Hydrogen internals yet to work with output in import or export.

lgeiger

@kylebarron this looks, good. I'm happy to add this functionality to Hydrogen.

Could you add some documentation in the readme?

lgeiger · 2018-12-30T18:34:03Z

lib/code-manager.js

+    return "code";
+  }
+  var rowText = getRow(editor, start.row - 1);
+  if (_.includes(rowText, "md") || _.includes(rowText, "markdown")) {


I think that's OK for now.

lgeiger · 2018-12-30T18:34:59Z

lib/code-manager.js

+  _.forEach(lines, line => {
+    if (line.startsWith(commentStartString)) {
+      // Remove comment from start of line
+      editedLines.push(line.slice(commentStartString.length).trimLeft());


Does this still allow for indentation in the markdown cell?

You're right, I don't think it does.

Just removing .trimLeft() isn't my favorite solution because then if a markdown cell looks like

# %% markdown # # Header # **bold** # - list # - indented list

then the text in the markdown cell in the Jupyter notebook would still have a leading space in each row, i.e.:

# Header **bold** - list - indented list

I think writing with a space after the # is clearer in the .py file, while having a leading space on every line in the exported markdown cell is annoying.

I added code to dedent common leading whitespace for Markdown blocks, so that the above cell would be converted to

# Header **bold** - list - indented list

I discovered the tiny dependency https://github.com/sindresorhus/strip-indent, which probably has better code than mine. I added that and removed my custom code.

lib/store/index.js

kylebarron · 2018-12-31T06:34:32Z

spec/store/index-spec.js

@@ -307,13 +307,15 @@ describe("Store", () => {
      expect(store.notebook).toEqual(commutable.toJS(nb));
    });

-    xit("should return a fully-fledged notebook when the file isn't empty", () => {


Previously the test had used xit(), so I think the test was never run, especially because it would have failed.

This was marked as expect fail intentionally because the error didn't make sense and we didn't want to hold up ci.

Ohh that makes sense. Is the updated test ok now?

kylebarron · 2018-12-31T06:35:23Z

spec/store/index-spec.js

@@ -307,13 +307,15 @@ describe("Store", () => {
      expect(store.notebook).toEqual(commutable.toJS(nb));
    });

-    xit("should return a fully-fledged notebook when the file isn't empty", () => {


The test still doesn't pass, but I'm not sure why because it seems to produce the correct output when I paste the test commands into the dev console.

You seem to have discovered why it was failing and made it pass, though It looks like we are testing that there is an empty cell at the top now, which seems like a bug. I know you pointed this out before, is it something to fix?

I updated that in #1505 https://github.com/nteract/hydrogen/pull/1505/files

…ort-notebook-markdown

kylebarron · 2019-01-03T05:22:02Z

@lgeiger It took a while but I figured out why the test wasn't passing. I think I had to activate language-python. I borrowed waitAsync from code-manager-spec.js. If you want me to create a separate file instead of duplicating the waitAsync wrapper, I can do that.

I also added one simple test for testing that code under %% markdown should be exported to a Markdown cell. Is that ok by itself?

Maybe tomorrow I'll add some documentation.

BenRussert · 2019-01-05T14:54:02Z

Maybe tomorrow I'll add some documentation.

I'll add the WIP tag for now, but remove it when you are ready and we can merge.

kylebarron · 2019-01-05T20:30:36Z

@BenRussert I added a new page in the documentation for exporting a file to a notebook. Hopefully in the future that page will also be updated for notebook import #1501

kylebarron · 2019-01-05T20:32:01Z

I also updated the breakpoints grammar so that markdown cell markers also get highlighted, at least if not using tree-sitter (#1134 (comment))

kylebarron · 2019-01-07T16:57:33Z

@BenRussert @lgeiger
If I have an approving review and some tests and they pass, is it ok for me to merge my own pull requests? I don't know what the acceptable norms are around that, and I don't want to commit any changes that aren't accepted.

BenRussert · 2019-01-07T19:38:43Z

Thanks @kylebarron!

BenRussert · 2019-01-14T22:10:13Z

Nice, looks like the test is correct there. Thanks for digging into that!

…

On Mon, Jan 14, 2019, 2:34 PM Kyle Barron ***@***.*** wrote: ***@***.**** commented on this pull request. ------------------------------ In spec/store/index-spec.js <#1498 (comment)>: > @@ -307,13 +307,15 @@ describe("Store", () => { expect(store.notebook).toEqual(commutable.toJS(nb)); }); - xit("should return a fully-fledged notebook when the file isn't empty", () => { I updated that in #1505 <#1505> https://github.com/nteract/hydrogen/pull/1505/files — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1498 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKW4cTzArOeAWb1cDczd9sBReTsKWjGCks5vDPf0gaJpZM4ZgvFD> .

Support exporting markdown cells to notebook file

1cf3353

kylebarron requested a review from BenRussert December 25, 2018 01:32

kylebarron mentioned this pull request Dec 28, 2018

Import Jupyter Notebook files into Atom #1501

Merged

lgeiger approved these changes Dec 30, 2018

View reviewed changes

Kyle Barron added 3 commits December 30, 2018 22:59

Dedent common whitespace in markdown cells

b699483

Use let instead of var

2fc9fd6

Attempt to fix test

4b2d859

kylebarron commented Dec 31, 2018

View reviewed changes

kylebarron mentioned this pull request Dec 31, 2018

Remove initial empty cell from codeManager.getCells() #1505

Merged

Kyle Barron added 4 commits December 31, 2018 13:16

Use strip-indent to dedent common whitespace in md cells

df8cf3e

Merge branch 'master' of https://github.com/nteract/hydrogen into exp…

4644bf9

…ort-notebook-markdown

Fix existing test

7366e3b

Add markdown export test

71fbd3e

BenRussert added the work in progress 👷‍♂️ label Jan 5, 2019

Kyle Barron added 3 commits January 5, 2019 14:20

Allow <md> and <markdown> as Markdown cell headers

e503054

Add Markdown cells to breakpoints highlighting

8e86739

Add Notebook Export documentation

6cee8f0

kylebarron removed the work in progress 👷‍♂️ label Jan 5, 2019

kylebarron mentioned this pull request Jan 6, 2019

Idea: Create ipynb notebook file from Hydrogen #801

Closed

BenRussert merged commit 2814771 into nteract:master Jan 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support exporting markdown cells to notebook file #1498

Support exporting markdown cells to notebook file #1498

kylebarron commented Dec 25, 2018 •

edited

Loading

kylebarron commented Dec 26, 2018

mwouts commented Dec 26, 2018

kylebarron commented Dec 26, 2018

mwouts commented Dec 26, 2018

kylebarron commented Dec 27, 2018

lgeiger left a comment

lgeiger Dec 30, 2018

lgeiger Dec 30, 2018

kylebarron Dec 30, 2018

kylebarron Dec 31, 2018

kylebarron Dec 31, 2018

kylebarron Dec 31, 2018

BenRussert Jan 14, 2019

kylebarron Jan 14, 2019

kylebarron Dec 31, 2018

BenRussert Jan 14, 2019

kylebarron Jan 14, 2019

kylebarron commented Jan 3, 2019 •

edited

Loading

BenRussert commented Jan 5, 2019

kylebarron commented Jan 5, 2019

kylebarron commented Jan 5, 2019

kylebarron commented Jan 7, 2019

BenRussert commented Jan 7, 2019

BenRussert commented Jan 14, 2019 via email

Support exporting markdown cells to notebook file #1498

Support exporting markdown cells to notebook file #1498

Conversation

kylebarron commented Dec 25, 2018 • edited Loading

kylebarron commented Dec 26, 2018

mwouts commented Dec 26, 2018

kylebarron commented Dec 26, 2018

mwouts commented Dec 26, 2018

kylebarron commented Dec 27, 2018

lgeiger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylebarron commented Jan 3, 2019 • edited Loading

BenRussert commented Jan 5, 2019

kylebarron commented Jan 5, 2019

kylebarron commented Jan 5, 2019

kylebarron commented Jan 7, 2019

BenRussert commented Jan 7, 2019

BenRussert commented Jan 14, 2019 via email

kylebarron commented Dec 25, 2018 •

edited

Loading

kylebarron commented Jan 3, 2019 •

edited

Loading