👌 IMPROVE: Use configured nb_render_plugin for glue nodes

[bryanwweber]

In the process of creating an extension to render Bokeh outputs, I found that the CellOutputBundleNode was always using the default renderer. The fix is to set the renderer to the user-configured value.

I wasn't sure if a similar change should be made to the inline-type nodes.

[bryanwweber]

FYI, my extension is now publicly available on PyPI: https://pypi.org/project/myst-nb-bokeh/ and ReadTheDocs: https://myst-nb-bokeh.readthedocs.io/en/stable/

I'd appreciate a review of this PR so that I don't have to install MyST-NB from a branch in my fork 😄 Thanks!

[damianavila]

This seems a sensible request, boosting the signal 😉
@choldgraf, @chrisjsewell, WDYT?

[choldgraf]

It seems correct to make this configuration work like that. So if a plugin is used, is it meant to strictly extend the default behavior? Or is that environment variable expected to be set on a per notebook level or something? Maybe I'm not understanding how this particular extension point works well enough

Also just an FYI the docs failures are because the Myst docs changed, not because of this PR, but we should fix that on master

[codecov]

Codecov Report

Merging #337 (15ad46a) into master (2baade0) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #337   +/-   ##
=======================================
  Coverage   87.35%   87.35%           
=======================================
  Files          12       12           
  Lines        1368     1368           
=======================================
  Hits         1195     1195           
  Misses        173      173           

Flag	Coverage Δ
pytests	87.35% <100.00%> (ø)

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

Impacted Files	Coverage Δ
myst_nb/nb_glue/__init__.py	100.00% <ø> (ø)
myst_nb/nodes.py	100.00% <ø> (ø)
myst_nb/nb_glue/domain.py	90.64% <100.00%> (ø)
myst_nb/nb_glue/utils.py	73.01% <100.00%> (ø)

---

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 2baade0...15ad46a. Read the comment docs.

[bryanwweber]

It seems correct to make this configuration work like that. So if a plugin is used, is it meant to strictly extend the default behavior? Or is that environment variable expected to be set on a per notebook level or something? Maybe I'm not understanding how this particular extension point works well enough

The way I interpreted the requirement for a plugin is that you have to provide a rendering function for all of the mimetypes that are present in your documents. Whether that comes by extending the built-in rendering class, or writing everything from scratch, is immaterial as long as all the mimetypes are handled. I'm not sure that's correct though. This setting is configured globally in conf.py, so there's not (currently) a way to change that from a single notebook, if I understand correctly.

Also just an FYI the docs failures are because the Myst docs changed, not because of this PR, but we should fix that on master

Thanks, that's what I figured based on the error message but good to have confirmation 😊

[chrisjsewell]

Renderers and thus this logic will be significantly changing soon (in https://github.com/executablebooks/MyST-NB/tree/refactor-mystnb).
But let this merge this for now cheers, and I'll loop in you and explain everything when I create the PR for that

[welcome]

Congrats on your first merged pull request in this project! 🎉


Thank you for contributing, we are very proud of you! ❤️