Implement `NbGlueDomain` #73

chrisjsewell · 2020-03-14T08:39:52Z

closes #70 and also relates to #72

An initial draft of reworking the paste roles/directive into a domain. The domain achieve's two things:

It collates all roles/directives relating to rendering of 'glue' objects. So all you need to have in the extension setup is app.add_domain(NbGlueDomain).
It centralises access to the glue cache. This has allowed me, for example, to also track the notebook from where each glue key comes from, and remove keys related to a deleted document (see NbGlueDomain.clear_doc)

I have then added the PasteFigure directive as an example of extending the functionality of the domain for handling different output types. It allows for captions, labelling/referencing, width and alignment.
Take a look at:
https://173-241268229-gh.circle-artifacts.com/0/html/use/glue.html#using-the-nb-figure-directive
😄

Here we create a specific `PasteTextNode`, and move most of the logic for formatting etc out of the `PasteNodesToDocutils` transform. This makes the transform more generic, so that we can add more PasteNode subclasses to it in the future. We also rename the role from `nb:paste` to `nb:text`, to reflect its specific nature, and rewrite the documentation to reflect this.

chrisjsewell · 2020-03-14T18:22:32Z

In d4aa616, I have refactored the paste role and rewritten the documentation accordingly:
https://178-241268229-gh.circle-artifacts.com/0/html/use/glue.html#pasting-glued-variables-into-your-page

choldgraf · 2020-03-14T18:28:19Z

this looks awesome - I'll take a look soon!

chrisjsewell · 2020-03-14T18:28:22Z

I was also thinking of changing nb:paste to nb:output. I'm not fixed on the naming though and am open to other suggestions.

choldgraf · 2020-03-14T18:56:12Z

took a look at the docs - the figure bit is super cool!!

A few quick thoughts:

I wonder if the nb: prefix will be clear to people. It seems reasonable to me, but we should shop that around with the team to see if it is intuitive for them. Other options I can think about just for brainstorming purposes:
- glu:
- myst:
I kind like paste over output because I think it is more specific. calling it nb:output makes me assume it will be the output of a cell (which, one day, we may want to explicitly for cell outputs?).
For the nb:text role, could this be nb:paste as well, with the assumption that by default, anything pasted in-line will be pasted as text?
- To give myself a counter-point. I suppose (one day) we could try to add in-line options for things like in-line images (e.g., for sparklines) or for in-text HTML (like tangle.js)

chrisjsewell · 2020-03-14T19:01:04Z

To give myself a counter-point. I suppose (one day) we could try to add in-line options for things like in-line images

Yeh that was my thinking, that this role (with the formatting aspect) is very specific to text

choldgraf · 2020-03-14T19:05:22Z

to further prove my counter point, I tried adding ![](logo.png) in-line to some parts of the docs, and this is what it looked like:

I think this means we can basically already include "sparkline"-style images easily. That is super cool. All we'd need to do is insert that in the doctree and add a css rule like img.glue_inlue {max-height: 1em}

chrisjsewell · 2020-03-14T19:05:32Z

Yeh maybe glu is more descriptive: glu:text, glu:figure. In this context glu:paste doesn't really make sense, so maybe glu:object?

choldgraf · 2020-03-14T19:07:04Z

@chrisjsewell yeah I was thinking that if we used glu: then we wouldn't need paste

chrisjsewell · 2020-03-14T19:14:01Z

@chrisjsewell yeah I was thinking that if we used glu: then we wouldn't need paste

nb:paste -> glu:? (glu:object, glu:block, glu:any ?)

choldgraf · 2020-03-14T19:27:49Z

yeah - I was thinking then we'd always follow a glu:objecttype pattern.

Or maybe pst: for "paste"? We could register both pst: and paste: domains?

Edit: I wonder if domains can also be used as a role/directive as well? E.g. could glu: or pst: be used as a kind of glu:any drop-in?

chrisjsewell · 2020-03-14T20:10:23Z

We could register both pst and paste domains?

No I think only one domain name is allowed

chrisjsewell · 2020-03-14T20:16:42Z

Ok, for now, in 19b2b0e I have change the domain name to glu and changed nb:paste to glu:any.
I like any because it is reminiscent of the sphinx any reference role.

I have also added a glu:any (inline) role. This has required that I hack together an 'inline' version of cell_output_to_nodes:
https://github.com/ExecutableBookProject/MyST-NB/blob/19b2b0edf0b4c686f4d21eb9a6d40c1b3e119f7f/myst_nb/transform.py#L64

Obviously this should be up-streamed to jupyter-sphinx.

See https://179-241268229-gh.circle-artifacts.com/0/html/use/glue.html#pasting-glued-variables-into-your-page 😄

myst_nb/nb_glue/domain.py

choldgraf · 2020-03-14T20:53:06Z

myst_nb/nb_glue/domain.py

+    def create_node(self, output: dict):
+        """Create the output node, give the cell output."""
+        # the whole output chunk is deposited and rendered later
+        # TODO move these nodes to separate module, to avoid cyclic imports


I hope that we can just import these nodes from jupyter-sphinx at some point, whenever I can figure out the tests in jupyter/jupyter-sphinx#107

choldgraf · 2020-03-14T21:16:07Z

myst_nb/transform.py

@@ -49,3 +60,123 @@ def apply(self):
                continue
            col = ImageCollector()
            col.process_doc(self.app, node)
+
+
+def cell_output_to_nodes_inline(cell, data_priority, write_stderr, dir, thebe_config):


This one you'd like to upstream yeah? What's the main difference w/ jupyter-sphinx one?

Yep. All the nodes returned should be inline versions, i.e. literal_block -> literal, container -> inline, math_block -> math

choldgraf · 2020-03-14T21:16:52Z

This is looking really good to me - I like the API decisions in here, and we can always re-visit after others have had a chance to play with them.

myst_nb/nb_glue/domain.py

choldgraf · 2020-03-15T01:25:37Z

(FYI this is still in "draft" state but I am +1 to merge when you are happy with it)

So that e.g. tables inherit the same CSS

chrisjsewell · 2020-03-15T01:45:07Z

(FYI this is still in "draft" state but I am +1 to merge when you are happy with it)

Two last changes for you to review above, both were made so that pandas tables are formatted properly in figures here: https://189-241268229-gh.circle-artifacts.com/0/html/use/glue.html#tbl-df

chrisjsewell · 2020-03-15T01:52:51Z

If you do want to then merge, please add this as the commit message 😁

This commit introduces the NbGlueDomain for controlling glue outputs.
The domain achieves two things:

It collates all roles/directives relating to rendering of 'glue' objects. So all you need to have in the extension setup is app.add_domain(NbGlueDomain).
It centralises access to the glue cache, ensuring that keys related to deleted documents are removed

Multiple glue roles/directives have been introduced for formatting the output, including the PasteFigure directive, which allows for captions, labelling/referencing, width and alignment.

…in the parser

chrisjsewell · 2020-03-15T04:58:06Z

Final piece of the puzzle: https://194-241268229-gh.circle-artifacts.com/0/html/use/glue.html#the-glu-math-directive

choldgraf · 2020-03-15T05:36:00Z

This looks great to me - let's give it a merge and see how things feel as folks on the team use this, I am excited!

chrisjsewell added 13 commits March 14, 2020 04:21

Move glue to submodule

bd63a47

Separate glue code into more modules

7f29221

Move env.glue_data to domaindata

c2eeb7a

Centralise access to the glue cache through NbGlueDomain

85873fc

Improve glue caching

7a30323

add TODO

bd910f9

improve wrning message

eecdcfd

Add text for NbGlueDomain.clear_doc

ef978ba

fix typo

45ac546

Add PasteFigure

1779f31

add TODO

99065e2

add TODO

eb8dd00

Add referencing of PasteFigure

f49b053

chrisjsewell requested a review from choldgraf March 14, 2020 09:34

chrisjsewell added 3 commits March 14, 2020 18:38

DOC: Add a note on the remove_output tag

8623a73

minor doc fix

d62ed1f

minro doc fix

83f52b1

Rename NbGlueDomain to glu, add paste_any_role

19b2b0e

add TODO

b38cec4

choldgraf reviewed Mar 14, 2020

View reviewed changes

myst_nb/nb_glue/domain.py Outdated Show resolved Hide resolved

choldgraf reviewed Mar 14, 2020

View reviewed changes

PasteInlineNode: wrap inline in span and add CSS

306c406

choldgraf reviewed Mar 14, 2020

View reviewed changes

Add glu:any role to tests

1f88b44

chrisjsewell commented Mar 14, 2020

View reviewed changes

myst_nb/nb_glue/domain.py Outdated Show resolved Hide resolved

chrisjsewell added 5 commits March 14, 2020 22:11

Don't store images in the cache, if already on disc

c0a530c

remove TODO

dbabc73

minor update

8221c59

add test for sphinx setup

cad9c84

Improve documentation

0642eb5

chrisjsewell mentioned this pull request Mar 14, 2020

Add some instruction on modifying CSS styling in docs #76

Open

choldgraf mentioned this pull request Mar 15, 2020

FEAT: Jupyter-cache integration #55

Merged

4 tasks

Wrap glued CellOutputBundleNode in CellOutputNode

ac76fbc

So that e.g. tables inherit the same CSS

chrisjsewell marked this pull request as ready for review March 15, 2020 01:53

chrisjsewell added 5 commits March 15, 2020 03:07

fix bug whereby 'displayed' glue variables had the GLUE_PREFIX added …

be2dbb2

…in the parser

Fix test

0bd7f4c

Add glu:math directive

2376307

minor doc update

d88241b

typo fix

986db2f

choldgraf merged commit 4d83e8c into master Mar 15, 2020

choldgraf deleted the paste-domain branch March 15, 2020 05:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `NbGlueDomain` #73

Implement `NbGlueDomain` #73

chrisjsewell commented Mar 14, 2020 •

edited

chrisjsewell commented Mar 14, 2020 •

edited

choldgraf commented Mar 14, 2020

chrisjsewell commented Mar 14, 2020

choldgraf commented Mar 14, 2020

chrisjsewell commented Mar 14, 2020

choldgraf commented Mar 14, 2020 •

edited

chrisjsewell commented Mar 14, 2020

choldgraf commented Mar 14, 2020

chrisjsewell commented Mar 14, 2020 •

edited

choldgraf commented Mar 14, 2020 •

edited

chrisjsewell commented Mar 14, 2020

chrisjsewell commented Mar 14, 2020 •

edited

choldgraf Mar 14, 2020

choldgraf Mar 14, 2020

chrisjsewell Mar 14, 2020 •

edited

choldgraf commented Mar 14, 2020

choldgraf commented Mar 15, 2020

chrisjsewell commented Mar 15, 2020 •

edited

chrisjsewell commented Mar 15, 2020

chrisjsewell commented Mar 15, 2020

choldgraf commented Mar 15, 2020

Implement NbGlueDomain #73

Implement NbGlueDomain #73

Conversation

chrisjsewell commented Mar 14, 2020 • edited

chrisjsewell commented Mar 14, 2020 • edited

choldgraf commented Mar 14, 2020

chrisjsewell commented Mar 14, 2020

choldgraf commented Mar 14, 2020

chrisjsewell commented Mar 14, 2020

choldgraf commented Mar 14, 2020 • edited

chrisjsewell commented Mar 14, 2020

choldgraf commented Mar 14, 2020

chrisjsewell commented Mar 14, 2020 • edited

choldgraf commented Mar 14, 2020 • edited

chrisjsewell commented Mar 14, 2020

chrisjsewell commented Mar 14, 2020 • edited

choldgraf Mar 14, 2020

Choose a reason for hiding this comment

choldgraf Mar 14, 2020

Choose a reason for hiding this comment

chrisjsewell Mar 14, 2020 • edited

Choose a reason for hiding this comment

choldgraf commented Mar 14, 2020

choldgraf commented Mar 15, 2020

chrisjsewell commented Mar 15, 2020 • edited

chrisjsewell commented Mar 15, 2020

chrisjsewell commented Mar 15, 2020

choldgraf commented Mar 15, 2020

Implement `NbGlueDomain` #73

Implement `NbGlueDomain` #73

chrisjsewell commented Mar 14, 2020 •

edited

chrisjsewell commented Mar 14, 2020 •

edited

choldgraf commented Mar 14, 2020 •

edited

chrisjsewell commented Mar 14, 2020 •

edited

choldgraf commented Mar 14, 2020 •

edited

chrisjsewell commented Mar 14, 2020 •

edited

chrisjsewell Mar 14, 2020 •

edited

chrisjsewell commented Mar 15, 2020 •

edited