Add support for workflow code cells

[glopesdev]

This PR introduces a custom Sphinx extension and a new JS module for the generated website to enable rendering workflow containers. The script mimics the main features of the DocFX version, but reuses heavily the sphinx-copybutton extension.

Specifically, the markdown syntax for inserting a workflow code cell is identical to the DocFX version:

:::workflow
![Workflow](workflows/workflow.bonsai)
:::

The extension expects an SVG file with the same name to be located next to the workflow, e.g. the image for the workflow above is expected at workflows/workflow.svg.

Note

Due to quirks with JS working with local files, the fetch function to read contents only works if the website is being hosted from a webserver (i.e. not when opening HTML directly from files). This only affects the copy action and not the image rendering so I left it for now. Workaround is to preview the website by calling the built-in python webserver python -m http.server from the generated docs/html folder.

Fixes #68

[lochhh]

Thanks @glopesdev, cool stuff! I tried this out both locally and on my fork by copying the example in Bonsai.Sleap.PredictCentroids, but the URL seems to be broken. I made a suggestion to use relative URLs instead.

It would also be nice to have this documented in the contributing guidelines. Atm the docs guidelines only exist in this open PR. For now we should open an issue to remind us to do so once this feature is merged.

[glopesdev]

@lochhh I have rewritten the approach to use a custom extension for consistency with the image handling pipeline. Relative and deep paths should not be affected anymore, can you give your example another try?

P.S.: no need for the $ character anymore, just a normal image link is sufficient, I have updated the PR description.

[lochhh]

@lochhh I have rewritten the approach to use a custom extension for consistency with the image handling pipeline. Relative and deep paths should not be affected anymore, can you give your example another try?

P.S.: no need for the $ character anymore, just a normal image link is sufficient, I have updated the PR description.

Thanks @glopesdev this is a great idea and works well with .md. I'm not sure how it works with .rst files though. In my mind I would like to have for

.rst files:

.. workflow:: ../../workflows/CentroidModel.bonsai
   :alt: PredictCentroids

and .md files:

:::{workflow} ../../workflows/CentroidModel.bonsai
   :alt: PredictCentroids
:::

Here is my attempt at adapting your WorkflowImageConverter for a custom WorkflowDirective (2bbc53b).

[glopesdev]

@lochhh I understand the idea but are we going to have people write documentation directly in rst syntax?

I would actually actively discourage this, since rst is only a standard in python, whereas markdown is more accessible.

The syntax proposed in this PR is identical between docfx markdown and sphinx markdown, so I feel it might be best to unify how we document bonsai code blocks between docfx and sphinx than to force md to be compatible with rst.

Ultimately all MyST code must have an equivalent in rST. Do you know what would be the .rst equivalent of the current .md syntax?

[glopesdev]

@lochhh actually it looks like admonitions in MyST now support colon fences without curly brackets (executablebooks/MyST-Parser#271) which seem to be common across markdig, pandoc and now myst.

[lochhh]

@lochhh I understand the idea but are we going to have people write documentation directly in rst syntax?

I would actually actively discourage this, since rst is only a standard in python, whereas markdown is more accessible.

The syntax proposed in this PR is identical between docfx markdown and sphinx markdown, so I feel it might be best to unify how we document bonsai code blocks between docfx and sphinx than to force md to be compatible with rst.

Thanks again @glopesdev. I totally agree with your point of having a consistent markdown syntax between docfx and sphinx. From what I understand, the current approach is that we place an "image" in a custom "workflow" container and manipulate this image via the WorkflowImageConverter, and then workflow.js finds the matching container and adds the copy button. What I'm unsure about is implementing this as an ImageConverter, as something like the below (i.e. without the workflow container) would still render the image, only without the copy button:

![PredictCentroids](../workflows/CentroidModel.bonsai)

My suggestion for a directive is to have something similar to embedding mermaid diagrams: instead of providing mermaid file/code, we provide the workflow file, and the image would be rendered. I learnt from sphinx's mermaid extension that setting myst_fence_as_directive = ["workflow"] would allow us to use the workflow directive as a markdown code block (see updated preview):

```workflow
../workflows/CentroidModel.bonsai
```

This syntax is also similar to that for docfx's mermaid diagram. With a directive, we also get a similar RST syntax "for free" (even if we don't need it).
That said, I'm aware that much more work has been done on the bonsai docfx side than here - it is easier to directly adopt the current way of documenting bonsai code blocks, than reinventing the wheel - was fun to try it out anyway 😁

@lochhh actually it looks like admonitions in MyST now support colon fences without curly brackets (executablebooks/MyST-Parser#271) which seem to be common across markdig, pandoc and now myst.

The closing comment suggests the curly braces are necessary to distinguish between highlighting languages (```python) and directives(```{python}).

Ultimately all MyST code must have an equivalent in rST. Do you know what would be the .rst equivalent of the current .md syntax?

The rst equivalent for the current custom container syntax would be:

.. container:: workflow

    .. raw:: html

        <p>

    .. image:: ../workflows/CentroidModel.bonsai
        :alt: Alternative text

    .. raw:: html

        </p>

(☝️ This definitely gives us another reason to discourage documenting bonsai workflows in rst.)
as opposed to the directive approach:

.. workflow::
    :alt: Alternative text
    
    ../workflows/CentroidModel.bonsai

to get to:

<div class="workflow docutils container">
   <p>
      <img alt="Alternative text" src="../_images/CentroidModel.svg">
   </p>
</div>

[glopesdev]

The closing comment suggests the curly braces are necessary to distinguish between highlighting languages (```python) and directives(```{python}).

Thanks @lochhh, I guess I misread this comment and missed the double negation. That is unfortunate, maybe there isn't then a good way to unify things as they are.

My suggestion for a directive is to have something similar to embedding mermaid diagrams: instead of providing mermaid file/code, we provide the workflow file, and the image would be rendered. I learnt from sphinx's mermaid extension that setting myst_fence_as_directive = ["workflow"] would allow us to use the workflow directive as a markdown code block

This is not a bad idea, but it would require changes to our current shared docfx infrastructure for bonsai, which would potentially break or impact all the existing documentation sites. Definitely not impossible, but we are already stretched quite thin in terms of capacity so I am really hesitant to pick another hard battle.

Do we really foresee writing documentation directly in rST? Might there be some other way to generate a more pleasant syntax which would work across both MyST and rST? I was wondering perhaps we can implement both a converter and a directive, where one calls the other.

I agree none of this is necessarily pretty, but I am also wary of postponing a solution for longer, since we need a solution to integrate the documentation from @NeuroThom. I am also wary of changing the current syntax since then we will be accumulating syntax debt which will probably come back to haunt us later if we ever want to port this into another system.

To clarify, I am not opposed to changing our current syntax, but in that case we should probably first open an issue in https://github.com/bonsai-rx/docfx-tools to evaluate how the proposed syntax might work in docfx. When we get to a working syntax, we will still need to support the previous one for backward compatibility purposes anyway, so effort would not be wasted.

P.S.: e.g. where I foresee there might be questions is re. the syntax inside the code fence: in mermaid diagrams what is inside the fence is actual code rather than file paths. I see this more as a custom markdown preprocessor rather than a representation of code, which is why I went for the image inside a container approach rather than the code fence.

What I want to make sure here is that whatever we do we don't block ourselves in case we ever want to have proper code fence support for bonsai syntax (i.e. where you would place directly workflow code inside the block).

[glopesdev]

My suggestion for a directive is to have something similar to embedding mermaid diagrams: instead of providing mermaid file/code, we provide the workflow file, and the image would be rendered

@lochhh As a last defense of the image converter approach, I guess I was originally seeing this problem as a conversion between the .bonsai file format and SVG files. In the current implementation this is done by assuming the exported file lives right next to the source file, but actually in current bonsai doc sites we export the SVG files directly from source code, so in the future this could literally be a conversion process where we take the workflow file and output the SVG without checking it into version control.

[glopesdev]

The rst equivalent for the current custom container syntax would be:

I actually have a question about the rst syntax. Have you tried whether the following would work?

.. container:: workflow
    .. image:: ../workflows/CentroidModel.bonsai
        :alt: Alternative text

[lochhh]

The rst equivalent for the current custom container syntax would be:

I actually have a question about the rst syntax. Have you tried whether the following would work?

.. container:: workflow

    .. image:: ../workflows/CentroidModel.bonsai
        :alt: Alternative text

Actually, the above would work if we change this line as follows:

Suggested change

	imgParent.remove();
	// remove the parent p element, if it exists
	if (imgParent.nodeName.toLowerCase() === "p") {
	imgParent.remove();
	}

Because in .md the custom container places the <img> within <p>, i.e. <div><p><img>...</img></p></div>, whereas in .rst we'd get <div><img>...</img></div>, and removing the parent element of the img means we remove the entire <div>.

[lochhh]

Happy to merge this PR once the above is resolved, and perhaps revisit the syntax discussion later. Nonetheless, it's nice to know the reasoning behind the chosen syntax, as I thought of it more as "code that one can copy", than a special kind of image that needs to be converted.