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

Support headings in RST to MD #723

Merged
merged 6 commits into from Jul 7, 2020
Merged

Conversation

sdhiscocks
Copy link
Contributor

This enables support for converting headings from RST into Markdown text for Jupyter Notebooks, with heading levels maintained for entire notebook.

This enables support for converting headings from RST into Markdown text
for Jupyter Notebooks, with heading levels maintained for entire
notebook.
@codecov
Copy link

codecov bot commented Jul 1, 2020

Codecov Report

Merging #723 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #723      +/-   ##
==========================================
+ Coverage   97.47%   97.49%   +0.01%     
==========================================
  Files          32       32              
  Lines        3560     3587      +27     
==========================================
+ Hits         3470     3497      +27     
  Misses         90       90              
Impacted Files Coverage Δ
sphinx_gallery/notebook.py 97.59% <100.00%> (+0.15%) ⬆️
sphinx_gallery/tests/test_notebook.py 100.00% <100.00%> (ø)

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 0022f0e...87dca05. Read the comment docs.

Copy link
Contributor

@lucyleeow lucyleeow left a comment

Choose a reason for hiding this comment

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

This would be useful. I just have some nitpicks. Would be good for someone else to review as this was complicated!

sphinx_gallery/notebook.py Outdated Show resolved Hide resolved
sphinx_gallery/notebook.py Outdated Show resolved Hide resolved
r'(?P<pre>\A|^[ \t]*\n)' # Start of string or blank line above
r'(?:(?P<over>[{0}])(?P=over)*\n[ \t]*)?' # Over, with heading space
r'(?P<heading>\S[^\n]*)\n' # Heading itself
r'(?P<under>(?(over)(?P=over)|[{0}]))(?P=under)*$' # if over make same
Copy link
Contributor

Choose a reason for hiding this comment

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

'if over matched, make same' ?

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 was battling with flake8 here a bit. Maybe I should interweave comments, or add flake8 ignore?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, adding # noqa E501 to the end of the line (after the comment) fixes it. It looks messy but I think its okay (unless @larsoner objects)

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 decided to interweave the comments with the regular expression, as I thought it worked out better and allowed more verbose comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks that's clearer. Will help when we look back at this regex!

sdhiscocks and others added 3 commits July 4, 2020 17:45
[ci skip]

Co-authored-by: Lucy Liu <jliu176@gmail.com>
[ci skip]

Co-authored-by: Lucy Liu <jliu176@gmail.com>
Copy link
Contributor

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

@lucyleeow feel free to merge if you're happy

@lucyleeow
Copy link
Contributor

Thanks @sdhiscocks

@lucyleeow lucyleeow merged commit 3381f7a into sphinx-gallery:master Jul 7, 2020
@sdhiscocks sdhiscocks deleted the headings branch July 8, 2020 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants