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

Add support for workflow code cells #80

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add support for workflow code cells #80

wants to merge 3 commits into from

Conversation

glopesdev
Copy link
Collaborator

@glopesdev glopesdev commented Nov 20, 2024

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

@glopesdev glopesdev added the feature New planned feature label Nov 20, 2024
Copy link
Collaborator

@lochhh lochhh left a comment

Choose a reason for hiding this comment

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

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.

src/_static/js/workflow.js Outdated Show resolved Hide resolved
@glopesdev
Copy link
Collaborator Author

glopesdev commented Dec 18, 2024

@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.

@glopesdev glopesdev requested a review from lochhh December 18, 2024 14:01
@lochhh
Copy link
Collaborator

lochhh commented Dec 20, 2024

@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
Copy link
Collaborator Author

glopesdev commented Dec 20, 2024

@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
Copy link
Collaborator Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New planned feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port workflow copy and paste functionality to Sphinx markdown syntax
2 participants