[MRG] EHN support #%% cell separators by lucyleeow · Pull Request #518 · sphinx-gallery/sphinx-gallery

lucyleeow · 2019-07-11T09:31:42Z

closes #370

Support # %% as cell separators.
Amend 'test_bug_cases_of_notebook_syntax.py' to test 2 input docs - the original one that uses only #'s as a cell separator and one that uses both #'s and # %% as cell separators.
Amend docs to advise that # %% can also be used.

codecov-io · 2019-07-11T11:24:30Z

Codecov Report

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

@@            Coverage Diff             @@
##           master     #518      +/-   ##
==========================================
+ Coverage   96.23%   96.24%   +<.01%     
==========================================
  Files          29       29              
  Lines        2686     2687       +1     
==========================================
+ Hits         2585     2586       +1     
  Misses        101      101

Impacted Files	Coverage Δ
sphinx_gallery/py_source_parser.py	`91.76% <ø> (ø)`	⬆️
sphinx_gallery/tests/test_gen_rst.py	`98.75% <100%> (ø)`	⬆️

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 f2e6cc9...b54e8f4. Read the comment docs.

agramfort · 2019-07-11T11:33:01Z

+====================
+
+When writting latex in a Python string keep in mind to escape the backslashes
+or use a raw docstring


this file looks too much a copy paste from https://sphinx-gallery.github.io/tutorials/plot_parse.html
I would suggest to see how to integrate both so users find the information more easily.

my 2c

It IS a copy paste from that.
The original test I modified (test_bug_cases_of_notebook_syntax() from https://github.com/sphinx-gallery/sphinx-gallery/blob/master/sphinx_gallery/tests/test_gen_rst.py) tests that split_code_and_text_blocks() is performed correctly on the file 'tutorials/plot_parse.py'. I just amended that test to also test a version of 'plot_parse.py' ('plot_parse_both.py') which uses # %% as separators as well as #'s.

Sorry, what do you mean integrate both? I can change the test to test only the 'new' plot_parse_both.py file which uses both #'s and # %% as a separator?

now 2 almost identical files are going to show up on the documentation. I agree that it does the job for the testing but from a narrative doc perspective I would suggest this new example is as easy to discover as the old one OR to integrate them into one (at least for the documentation perspective).

my 2c

Ah I see the problem now. I will integrate them into one.

choldgraf · 2019-07-11T12:57:41Z

shit why didn't we think of this before haha, I think it's a good patch, seems reasonable to me. So if I wanted to add rST underneath it would look something like:

# %%
# my rst
# ======

my = "code"

?

agramfort · 2019-07-11T12:57:52Z



-##############################################################################
+# %%


I would add a note that at the top of the file that 2 types of separators are possible. Say you'll use
the ### at the beginning of the file, put a note saying that you'll use in what follows the alternative
approach with # %%

does it make sense?

Yes I realised above when you talked about the double up that the plot_parse.py file is actually a page in the sphinx-gallery documentation (whoops). Am now amending this file to add the relevant information.

GaelVaroquaux

Looks good overall !

I think that an entry in the changelog is needed.

Also, it might be good to add somewhere a note on productivity tips (for instance using the sphinx directive "topic"), explaining to people how they can work in a convenient way bending these tools)

larsoner

Can you tweak this test:

https://github.com/sphinx-gallery/sphinx-gallery/blob/master/sphinx_gallery/tests/test_gen_rst.py#L105

By making the second set of ####### into # %%?

Otherwise LGTM!

lucyleeow · 2019-07-12T16:10:41Z

After speaking with @GaelVaroquaux we think both # %% and #%% (with and without space) should be supported:

Spyder supports both because Eclipse's automatic code formatting changes #%% into # %% (see this issue)
Hydrogen states that # %% is supported in their documentation but on testing both syntaxes work and exports correct cell formatting to jupyter notebook.
VSCode states that #%% is supported but on testing both syntaxes work.
Could not test in PyCharm as this functionality is only available in their professional version.

If there are no objections, I will change the code and documentation to support both syntaxes. I think I will state that a line of #'s or #%% can be used and just note that # %% is also acceptable for compatibility.

Will also add to the documentation a guide/tips to using the various IDE's with sphinx-gallery suggested by @GaelVaroquaux above.

larsoner · 2019-07-12T16:23:54Z

Works for me

…ate CHANGES

larsoner · 2019-07-12T17:55:28Z

I think that an entry in the changelog is needed.

I just noticed this bit. FYI we don't need changelog entries because we autogenerate them based on PR labels. But it's fine to leave the one you added in there

larsoner

Other than a tiny suggestion LGTM +1 for merge

Please remove WIP from the title if it is indeed ready to go

lucyleeow · 2019-07-15T12:15:05Z

Amended the doc to add a note on using sphinx gallery with code blocks. Hopefully this makes sense.

choldgraf · 2019-07-15T15:00:30Z

This seems good to me! @GaelVaroquaux you're still blocking changes just FYI :-)

larsoner · 2019-07-15T16:44:50Z

@GaelVaroquaux you're still blocking changes just FYI :-)

From the above I'm pretty sure it's just for the changelog update and link updating which are done. So I'll merge but if there was something still wrong @GaelVaroquaux feel free to chime in and we'll fix it.

Thanks @lucyleeow !

GaelVaroquaux · 2019-07-15T17:36:41Z

@GaelVaroquaux you're still blocking changes just FYI :-)

Aaaaaaarg!

From the above I'm pretty sure it's just for the changelog update and link updating which are done. So I'll merge but if there was something still wrong @GaelVaroquaux feel free to chime in and we'll fix it.

No, this is great. Thank you!

rth · 2020-04-28T09:54:12Z

Thanks for this @lucyleeow ! it is going to make scikit-learn examples much more readable in plain text mode scikit-learn/scikit-learn#17068

lucyleeow added 3 commits July 11, 2019 11:21

support # %% cell separators

5493dfe

amend plot parse file name and test comment

cc387a0

amend docs to specify # %% can be used also

152beba

lucyleeow changed the title ~~[WIP] EHN support # %% cell separators~~ [MRG] EHN support # %% cell separators Jul 11, 2019

lucyleeow added 2 commits July 11, 2019 13:10

format

5f9f898

rename plot parse back to original

3f11b72

agramfort reviewed Jul 11, 2019

View reviewed changes

run test on plot parse version with both separators only

3bf7ce8

agramfort reviewed Jul 11, 2019

View reviewed changes

lucyleeow changed the title ~~[MRG] EHN support # %% cell separators~~ [WIP] EHN support # %% cell separators Jul 11, 2019

GaelVaroquaux requested changes Jul 12, 2019

View reviewed changes

Comment thread doc/syntax.rst Outdated

larsoner reviewed Jul 12, 2019

View reviewed changes

lucyleeow changed the title ~~[WIP] EHN support # %% cell separators~~ [WIP] EHN support #%% cell separators Jul 12, 2019

support both syntaxes, update documentation, update and add test, upd…

af163c0

…ate CHANGES

larsoner reviewed Jul 12, 2019

View reviewed changes

Comment thread sphinx_gallery/tests/reference_parse.txt Outdated

larsoner added the enhancement label Jul 12, 2019

amend to comply with pep8

ce994a1

larsoner reviewed Jul 13, 2019

View reviewed changes

Comment thread sphinx_gallery/py_source_parser.py Outdated

larsoner reviewed Jul 13, 2019

View reviewed changes

lucyleeow added 2 commits July 15, 2019 11:35

more compact regex

59ba9a6

amend doc to add IDE tip

b54e8f4

lucyleeow changed the title ~~[WIP] EHN support #%% cell separators~~ [MRG] EHN support #%% cell separators Jul 15, 2019

clarify doc

abf5b7d

larsoner merged commit 46e1335 into sphinx-gallery:master Jul 15, 2019

lucyleeow mentioned this pull request Mar 6, 2020

Mention the multiple possible separators in the notebook-style example #611

Closed

lucyleeow deleted the IS/370 branch April 6, 2020 13:34

lucyleeow mentioned this pull request Apr 29, 2020

DOC simpler block delimitation in sphinx-gallery examples scikit-learn/scikit-learn#17068

Merged



		##############################################################################
		# %%

Conversation

lucyleeow commented Jul 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Jul 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

agramfort Jul 11, 2019

Choose a reason for hiding this comment

Uh oh!

lucyleeow Jul 11, 2019

Choose a reason for hiding this comment

Uh oh!

agramfort Jul 11, 2019

Choose a reason for hiding this comment

Uh oh!

lucyleeow Jul 11, 2019

Choose a reason for hiding this comment

Uh oh!

choldgraf commented Jul 11, 2019

Uh oh!

agramfort Jul 11, 2019

Choose a reason for hiding this comment

Uh oh!

lucyleeow Jul 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GaelVaroquaux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

lucyleeow commented Jul 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsoner commented Jul 12, 2019

Uh oh!

Uh oh!

larsoner commented Jul 12, 2019

Uh oh!

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

lucyleeow commented Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

choldgraf commented Jul 15, 2019

Uh oh!

larsoner commented Jul 15, 2019

Uh oh!

GaelVaroquaux commented Jul 15, 2019 via email

Uh oh!

rth commented Apr 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lucyleeow commented Jul 11, 2019 •

edited

Loading

codecov-io commented Jul 11, 2019 •

edited

Loading

lucyleeow Jul 11, 2019 •

edited

Loading

lucyleeow commented Jul 12, 2019 •

edited

Loading

lucyleeow commented Jul 15, 2019 •

edited

Loading

rth commented Apr 28, 2020 •

edited

Loading