[MRG] EHN support #%% cell separators #518
Conversation
Codecov Report
@@ Coverage Diff @@
## master #518 +/- ##
==========================================
+ Coverage 96.23% 96.24% +<.01%
==========================================
Files 29 29
Lines 2686 2687 +1
==========================================
+ Hits 2585 2586 +1
Misses 101 101
Continue to review full report at Codecov.
|
tutorials/plot_parse_both.py
Outdated
| ==================== | ||
|
|
||
| When writting latex in a Python string keep in mind to escape the backslashes | ||
| or use a raw docstring |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Ah I see the problem now. I will integrate them into one.
|
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"? |
tutorials/plot_parse.py
Outdated
| @@ -11,7 +11,7 @@ | |||
| Closing this string quotes on same line""" | |||
|
|
|||
|
|
|||
| ############################################################################## | |||
| # %% | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
|
After speaking with @GaelVaroquaux we think both
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 Will also add to the documentation a guide/tips to using the various IDE's with sphinx-gallery suggested by @GaelVaroquaux above. |
|
Works for me |
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 |
|
Amended the doc to add a note on using sphinx gallery with code blocks. Hopefully this makes sense. |
|
This seems good to me! @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 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!
|
|
Thanks for this @lucyleeow ! it is going to make scikit-learn examples much more readable in plain text mode scikit-learn/scikit-learn#17068 |
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.