ENH: Add gated directives to support code-cells for exercises and solutions

[mmcky]

This PR adds support for gated exercise and solution directives, which enables:

1. support for executable code (such as code-cell in myst-nb) to be used. As the code-cell is at the root level of the document it is executed by jupyter-cache and follows with jupyter-book methodology of supporting code-cell directives at the root level of the document in a linear fashion (which is 1-to-1 with jupyter)
2. supports any directives that may be used between [exercise,solution]-start and a [exercise,solution]-end directive by transforming the AST which can be a convenient way to add nested directives without needing to increase the back-tick depth

Basic Syntax

```{exercise-start}
:label: ex1
```

paired with

```{exercise-end}
```

The exercise-start directive allows for he same options as core exercise directive.

```{solution-start} ex1
```

paired with

```{solution-end}
```

Example

```{solution-start} exercise-1
:label: solution-gated-1
```

This is a solution to Exercise 1

```{code-cell} python3
import numpy as np
import matplotlib.pyplot as plt

# Fixing random state for reproducibility
np.random.seed(19680801)

dt = 0.01
t = np.arange(0, 30, dt)
nse1 = np.random.randn(len(t))                 # white noise 1
nse2 = np.random.randn(len(t))                 # white noise 2

# Two signals with a coherent part at 10Hz and a random part
s1 = np.sin(2 * np.pi * 10 * t) + nse1
s2 = np.sin(2 * np.pi * 10 * t) + nse2

fig, axs = plt.subplots(2, 1)
axs[0].plot(t, s1, t, s2)
axs[0].set_xlim(0, 2)
axs[0].set_xlabel('time')
axs[0].set_ylabel('s1 and s2')
axs[0].grid(True)

cxy, f = axs[1].cohere(s1, s2, 256, 1. / dt)
axs[1].set_ylabel('coherence')

fig.tight_layout()
plt.show()
```

With some follow up text to the solution

```{solution-end}
```

will be styled as a single solution in the output document

Technical Note

Currently the solution and exercise implementations differ in approach to try different ways of achieving the same thing.

1. The exercise approach maintains the original node type exercise or enumerate_exercise and instead uses gated=True/False metadata on the class to determine if it is an exercise start node. The benefit of this approach is that the content is more easily merged as you don't need to create a new_node and reassemble it (as in the case of solutions)
2. The solution approach uses fully separate wrapper nodes solution-start and solution-end, however the solution-start node needs to be cast into a solution node in the transform.

---

Discussions around Syntax

Pro:

1. This approach is independent of the current solution directive so doesn't change any existing behaviour
2. Looks quite clean from a syntax perspective (see example below)

Con:

1. Harder for highlighter tools to parse (i.e. vscode extension)
2. There are two directives that do a similar thing (solution and solution-start with solution-end). (@mmcky comment: Not sure this is a huge issue as I think users would typically use one or the other. I would probably start using the gated directives as I like how they look (rather than nested structures).

Technical Approach

This PR adds in two directives solution-start and solution-end which are then transformed during transform phase of sphinx to migrate any nodes between start and end to be wrapped in a standard solution node. Therefore there are no additional visit_ and depart_ methods required.

• solution-end can be simplified as it currently inherits from SolutionDirective and doesn't require all the additional logic of titles. It just needs to plan a solution-end node in the tree.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 96.82540% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.10%. Comparing base (92b9614) to head (a16787b).

Files with missing lines	Patch %	Lines
sphinx_exercise/directive.py	94.73%	3 Missing ⚠️
sphinx_exercise/transforms.py	97.32%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
+ Coverage   92.98%   94.10%   +1.12%     
==========================================
  Files           6        7       +1     
  Lines         442      628     +186     
==========================================
+ Hits          411      591     +180     
- Misses         31       37       +6     

Flag	Coverage Δ
pytests	94.10% <96.82%> (+1.12%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[choldgraf]

This seems like a pretty reasonable approach to me - I think we should document the "when to use one vs. the other" question strongly, and also make it clear that the functionality between the two is totally equivalent, the only difference is allowing people to nest {code-cell} objects within solution blocks.

Do you anticipate people wanting to have the same thing for exercise directives?

[choldgraf]

Why would anybody use :label: :class: etc here? I feel like we shouldn't document this pattern and should probably raise an error if somebody puts config here - it seems simpler to have all configuration in the start part of the directive

EDIT: ah I see you plan to do this :-)

[mmcky]

Indeed. Need to change solution-end to a much simpler class. The only complicated factor is that it can contain content so we need some state parsing etc.

[choldgraf]

Maybe it'd be good to do a quick check first and raise some errors for the following conditions:

• Accidentally nesting solution start:

  ```{solution-start} solution1
  ```
  ```{solution-start} solution2
  ```
  ```{solution-end}
  ```
  ```{solution-end}
  ```
  

• End solution without start solution

• Start solution without end solution (you have this one documented in the comments I believe)

[mmcky]

I agree. There needs to be a lot more robustness put into the code to check for structural issues and malformed syntax.

[mmcky]

Thanks @choldgraf that is a good test case 👍

[jstac]

This definitely works for me @mmcky . Fits my simple uses cases and the syntax is clean and simple enough to easily remember. Fully in favor 👍

Oops, hit wrong button 😬 -- reopened.

[mmcky]

Thanks for comments @jstac @choldgraf (@chrisjsewell via slack). Appreciate it.

Next Steps:

• do another round to improve robustness and write a more general tree parsing function for collecting relevant nodes between -start and -end.
• add developer documentation

There is the possibility to make this more general to extend any directives using {{directive}}-start and {{directive}}-end but won't put development time into this at the present time. Perhaps we could revisit that for an admonition factory type of project. I will document the general approach if others want to use this pattern in an extension.

[mmcky]

@AakashGfude I just pushed some additional test cases error.md but with the current test setup using app.build() I don't see a way to save the errors and warnings to pass to an assert statement. Any ideas on this?

Also the way the transform works currently is:

1. it parses each document looking for solution-start nodes
2. An exception is raised if no matching solution-end is raised when finding nodes to merge into a base solution node

I don't really see a reason to do any further structural testing on the document. This condition seems to be the main issue -- and I think an exception (vs. a warning) is required so users can fix the issue in the md.

• incorporate edge cases listed by @choldgraf above into error testing once testing infrastructure is setup correctly

[mmcky]

OK I think the only way to integrate proper error checking is to:

• add a transform CheckGatedDirectives to run through the full structure of the document and build a registry to ensure the structure of gated directives is complete (i.e. matching -start and -end), non-nestedness etc. and raise Errors if the required structure is not adhered to.

Alternatively,

• the registry can be formed as the directives are parsed.

I will try the alternative first as it would be more efficient to check an already formed registry (as part of the parsing step)

The current transform then doesn't have to know the structure of the document and can focus on migrating the appropriate nodes into a unified solution note in the sphinx.ast.

[psychemedia]

In a simple exercise solution, the dropdown class can be added to the solution to hide it.

For compound solutions between gated sections, it would be useful if the whole section could be collapsible, perhaps by the following:

```{solution-start} solution1
:class: dropdown
```

More generally, can a class be set in the solution-start block, eg :class: orange for custom styling, that is then automatically removed (by default, at least) by the matching solution-end block.

[psychemedia]

Regarding supporting code in exercise directives, this can be useful where some set up or initialisation code is required as part of the exercise set-up.

Generalising support for gated directives would seem to be a good way of doing this, not least because it then opens up a route for other extension developers to easily support high level blocks defined across multiple heterogenous cells.

Naively, I'm assuming that at some point the gated directives essentially gives you a <div> </div> block to put things in; as such, it would also be useful to allow the user to optionally set a CSS id for the block.

[mmcky]

In a simple exercise solution, the dropdown class can be added to the solution to hide it.

For compound solutions between gated sections, it would be useful if the whole section could be collapsible, perhaps by the following:

```{solution-start} solution1
:class: dropdown

More generally, can a class be set in the `solution-start` block, eg `:class: orange` for custom styling, that is then automatically removed (by default, at least) by the matching `solution-end` block.

Thanks @psychemedia I think this is a good idea. The way solution-start is setup at the moment is it inherits the solution directive so all options available to solution nodes should transfer over to solution-start. Maybe I need to add a test case as I work through the gated nodes.

[mmcky]

Regarding supporting code in exercise directives, this can be useful where some set up or initialisation code is required as part of the exercise set-up.

Support for exercise nodes will be next.

[mmcky]

@chrisjsewell @AakashGfude I have setup the extension to make use of a CheckGatedDirectives transform before merging them in the sphinx.AST to check for various structural issues. I am not sure how to get this setup working in the test framework though -- Is it possible to catch a sphinx ExtensionError and keep executing the document.

I want sphinx to stop executing if any of these malformed gated directive conditions exist:

1. no matching -end
2. no matching -start
3. nested sequence of -start and -end

coupled with a useful error message.

Currently the make html presents a nice diagnostic message for tests/books/test-gateddirective/

(ebp) ➜  test-gateddirective git:(gated-directive) make html
Running Sphinx v4.4.0
myst v0.15.2: MdParserConfig(renderer='sphinx', commonmark_only=False, enable_extensions=['dollarmath'], dmath_allow_labels=True, dmath_allow_space=True, dmath_allow_digits=True, dmath_double_inline=False, update_mathjax=True, mathjax_classes='tex2jax_process|mathjax_process|math|output_area', disable_syntax=[], url_schemes=['http', 'https', 'mailto', 'ftp'], heading_anchors=None, heading_slug_func=None, html_meta=[], footnote_transition=True, substitutions=[], sub_delimiters=['{', '}'], words_per_minute=200)
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 6 source files that are out of date
updating environment: [new config] 6 added, 0 changed, 0 removed
Executing: errors_1 in: /Users/mmcky/work/executablebooks/sphinx-exercise/tests/books/test-gateddirective                               
ERROR: The document (errors_1) is missing a solution-end directive
  solution-start at line: 20

Extension error:
[sphinx-exercise] An error has occured when parsing gated directives.
Please check warning messages above
make: *** [html] Error 2

but only for error_1 and I am not sure how to test against this condition and keep executing for errors_2 and errors_3 documents.

Should I run a sphinx.build() for each of the error pages individually and catch the exception?

[mmcky]

OK I have just found https://github.com/executablebooks/sphinx-external-toc/blob/96c80db479b9cd926f2d4dad9cc43ee3c007fcb7/sphinx_external_toc/events.py#L69 so maybe that is tested in sphinx-external-toc. I'll look there.

[psychemedia]

I'm trying this from pip3 install --upgrade git+https://github.com/executablebooks/sphinx-exercise.git@gated-directive but I don't see the code cells being displayed? Is code cell inclusion between the gateposts supported yet? Mangled Jupytext header code breaking code display.

[psychemedia]

Should the solution content blocks all be inside a solution-content div, which would then allow for collapsing the solution content?

[mmcky]

Should the solution content blocks all be inside a solution-content div, which would then allow for collapsing the solution content?

thanks @psychemedia -- I will double check the html. Is this just for gated solutions?

[psychemedia]

Is this just for gated solutions?

Yes, that's where I spotted it. Inside a gated solution. I was expecting the content between the gateposts to be in the solution-content div. Also, looking at it now, is the id="solution-content" guaranteed unique? It looks more like a class than an id to me.

[mmcky]

nicely spotted thanks @psychemedia the test fixture shows the same -- will fix that.

<div class="solution admonition" id="solution-gated-1">
<p class="admonition-title"><a class="reference internal" href="exercise.html#exercise-1">Solution to Exercise 1</a></p>
<div class="section" id="solution-content">
</div>
<p>This is a solution to Exercise 1</p>

• fix non-nested solution div in the translator phase
• check if id="solution-content" should be added to class="section solution-content"

The sphinx.ast is looking properly nested so this suggests it is the translation phase rather than transforms

<solution_node classes="solution" docname="solution" hidden="False" ids="solution-gated-1" label="solution-gated-1" names="solution-gated-1" serial_number="0" target_label="exercise-1" title="Solution to" type="solution">
            <solution_title>
                Solution to
            <section ids="solution-content">
            <paragraph>
                This is a solution to Exercise 1
            <CellNode cell_type="code" classes="cell">
                <CellInputNode classes="cell_input">
                    <literal_block language="ipython3" xml:space="preserve">

[psychemedia]

Presumably, the gated solutions will not be useful in JupyterLab / JupyerLab-MyST contest because of the inability to propagate metadata up the DOM / select parent HTML blocks that define cells; or, at least, not without some sort of extension support, or the availability of CSS has() selectors?

[psychemedia]

How easy is it to be able to parse a :class: dropdown on entering the gated section, and then add the dropdown button to the solution div?

More generally, can the recipe for gated sections be used to easily create other admonition types that also employ gated sections? Eg could gated blocks be easily added to exercise definitions, or other custom defined admonition blocks based on a similar pattern?

[mmcky]

Presumably, the gated solutions will not be useful in JupyterLab / JupyerLab-MyST contest because of the inability to propagate metadata up the DOM / select parent HTML blocks that define cells; or, at least, not without some sort of extension support, or the availability of CSS has() selectors?

Sadly no - to my knowledge this will need extension support. There is some work going on at mystjs to bring rendering of MyST support for jupyter/jupyterlab.

[mmcky]

OK so it looks like the xml tree is incorrect and the solution-content section should nest the content and it should be:

<section ids="solution-content">
            <paragraph>
                This is a solution to Exercise 1
            <CellNode cell_type="code" classes="cell">
                <CellInputNode classes="cell_input">
                    <literal_block language="ipython3" xml:space="preserve">

[mmcky]

• work out how to best support error_1, error_2 and error_3 tests

[mmcky]

@chrisjsewell @AakashGfude I think I figure out a reasonable solution in 701a398 for testing the Exception cases and checking warning messages

[mmcky]

@choldgraf @psychemedia @AakashGfude this is working nicely for me now.

I'll plan to merge this soon and start work on exercise directives.

@choldgraf this extension to the syntax is quite convenient (vs. nesting structures) -- it might be a nice enhancement proposal for any directive down the track.

A real world example can be seen here:

https://6226e59b74051d4c841f8092--epic-agnesi-957267.netlify.app/functions.html#solutions

which uses sphinx-togglebutton to collapse the blocks (thanks @choldgraf)

[mmcky]

I have now converted one of the QuantEcon lectures sites to use sphinx-exercise that makes use of both gated exercise and solution directives QuantEcon/lecture-python-programming.myst#176

The preview is available here: https://6228365c7ce7b477e5575a73--epic-agnesi-957267.netlify.app/intro.html

@AakashGfude would you mind to take a quick look and let me know what you think.