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

Literate programming #2592

Closed
wants to merge 3 commits into from
Closed

Literate programming #2592

wants to merge 3 commits into from

Conversation

Carreau
Copy link
Member

@Carreau Carreau commented Nov 16, 2012

Ability to show value of kernel variable in MD cell.

Do not view example with nbviewer, it does not support it yet.

allow to write code like

>>> a=1

then in MD cell a=::a:: which will be rendered as a=1

This does not allow repr_tex, or anything else as it uses user_variable.
Should we consider extending protocol ? Did I missed a clean way to do it ?

@bfroehle
Copy link
Contributor

@Carreau: Can you answer a naive question... I don't know anything about user_variable.

In your example above, is the value of a stored in the .ipydb file? If so, what is the representation? So we just store str(a) or repr(a) as as a string?

Would it be easy to extend the notebook viewer to support this?

@bfroehle
Copy link
Contributor

Also, in US English I think it would be literate, not literrate.

@Carreau
Copy link
Member Author

Carreau commented Nov 18, 2012

In your example above, is the value of a stored in the .ipydb file? If so, what is the representation? So we just store str(a) or repr(a) as as a string?

I store the repr in metadata.
from the doc

user_variables: If only variables from the user’s namespace are needed, a list of variable names can be passed and a dict with these names as keys and their repr() as values will be returned.

Would it be easy to extend the notebook viewer to support this?

Yes, that would be easy. I'm just thinking of extending ::x:: to ::Tex::x:: ::html::x:: for corresponding repr. (hence my question on messaging and user_variables) and the final syntax we would choose, to make the nbconvert modification.

Also nbviewer ships with a modified version of nbconvert, so I would like nbconvert to support conversion without going through files to dump the modified version which is a pain to maintain.

@Carreau
Copy link
Member Author

Carreau commented Nov 18, 2012

Also, in US English I think it would be literate, not literrate.

I was too lazy to look for it, I'll fix that. Thanks.

var text = this.get_text();
if (text === "") { text = this.placeholder; }
text = IPython.mathjaxutils.remove_math(text);
var literateRegex = /::([a-z_0-9]+)::/ig,
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't the correct regular expression for Python variables. They can't start with numbers.

There's also Python 3 Unicode variables, but I don't think anyone will blame you if you don't support those :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, also one can want to show the value of a dict, or a list, or an object property....

@asmeurer
Copy link
Contributor

Why do you use repr instead of str?

@Carreau
Copy link
Member Author

Carreau commented Nov 20, 2012

Why do you use repr instead of str?

Because that's what user_variables returns.
We might extend user_variables to return different kind of repr, we could include __str__ and __unicode__

I've started to think at how to extend user_varibles here

@Carreau
Copy link
Member Author

Carreau commented Nov 20, 2012

literrate -> literate

};


MarkdownCell.prototype = new TextCell();

MarkdownCell.prototype.set_litterate_data = function (data) {
Copy link
Member

Choose a reason for hiding this comment

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

litterate -> literate here as well

@ivanov
Copy link
Member

ivanov commented Nov 25, 2012

also, this doesn't seem to work for me (Chromium 22.0.1229.94 Debian wheezy/sid (161065))

@Carreau
Copy link
Member Author

Carreau commented Nov 25, 2012

ah.. I may have forgot to push one commit.

also, this doesn't seem to work for me (Chromium 22.0.1229.94 Debian wheezy/sid (161065))

There is no reason for it not to work... what does it not do ?
(dumb question, did you clear the browser cache ? )

@Carreau
Copy link
Member Author

Carreau commented Nov 25, 2012

Fixed and rebase to have the introduction of the example notebook in a separate commit.

I use the same version of chrome without problem, I don't see what could cause it not to work.

@asmeurer
Copy link
Contributor

It worked for me back when I tested it.

@ellisonbg
Copy link
Member

So I have a question: after the variable is replaced in a markdown cell, can it ever be updated if the kernel variable change, or do you have to go back and retype it later?

@asmeurer
Copy link
Contributor

You have to edit and re-render the Markdown cell. It's a little awkward since you can't execute Markdown cells like you can code cells.

@ellisonbg
Copy link
Member

But you have to remove to old kernel value, re-add the literate :: marks and then rerender it right?

@Carreau
Copy link
Member Author

Carreau commented Nov 28, 2012

But you have to remove to old kernel value, re-add the literate :: marks and then rerender it right?

No, just just have go go into edit mode and back. Once Code Mirror is shown even for a split second, the cell is marked as "dirty" and request new value to the kernel when re-rendered. (it is actually rendered once with empty placeholder and re-render one more time when values come back from the kernel).

From the few test I did "run all" should also update the MD cell, and in the right order as the MD kernel execute request are interleaved with the code cell execute request.... etc..

@filmor
Copy link
Contributor

filmor commented Nov 28, 2012

Wouldn't it be nice to have a variant of this that updated automatically by either querying the value from the kernel whenever a cell is run or getting the new value pushed from the whenever it changes? I'd like that :)

@Carreau
Copy link
Member Author

Carreau commented Nov 28, 2012

Wouldn't it be nice to have a variant of this that updated automatically by either querying the value from the kernel whenever a cell is run or getting the new value pushed from the whenever it changes? I'd like that :)

We've discussed that with @fperez at scipy, and we believe it is dangerous in the sens that you can change the value of e variable

x=1

x is equql to ::x::

x=x+1

now x is equal to ::x::

would be rendered as

x=1

x is equql to 2

x=x+1

now x is equal to 2

But yes, it is doable.

I'm thinking of embeding also the prompt number so that you could say:

when prompt number is :::prompt::: then x is ::x::

@asmeurer
Copy link
Contributor

Isn't the prompt number already available as a special variable?

@ellisonbg
Copy link
Member

Ohh, this is very nice. A few questions then:

  • Have we thought very carefully about the usage of the ::x:: syntax. We
    want to pick something that is a good match to markdown's syntax. What
    alternatives were considered?
  • Do we currently store both the original and rendered content for a
    markdown cell? Otherwise, we won't be able to capture this content in
    nbcovert logic.
  • But, even if we save the rendered HTML content, it won't help nbcovert in
    many cases, because it will use the original markdown in many cases. What
    we really need to do is save a dict of the keys/values that are used for
    that cell. Then those values can be used by nbcovert in whatever way is
    needed. Does this make sense?

On Wed, Nov 28, 2012 at 1:46 AM, Benedikt Sauer notifications@github.comwrote:

Wouldn't it be nice to have a variant of this that updated automatically
by either querying the value from the kernel whenever a cell is run or
getting the new value pushed from the whenever it changes? I'd like that :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/2592#issuecomment-10796351.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@Carreau
Copy link
Member Author

Carreau commented Nov 28, 2012

Isn't the prompt number already available as a special variable?

Yes, it in in metadata... bu if you have a real prompt_number variable what do you do ?

Have we thought very carefully about the usage of the :: syntax. We
want to pick something that is a good match to markdown's syntax. What
alternatives were considered?

I'm open to suggestion. ::something:: was easy to parse and sufficiently different from md/html/code, fast to type.

Do we currently store both the original and rendered content for a
markdown cell? Otherwise, we won't be able to capture this content in
nbcovert logic.

Actually at convert time you can render the html with phantom js as png and put it as an image in TeX... where is the problem ?

What we really need to do is save a dict of the keys/values that are used for
that cell. Then those values can be used by nbconvert in whatever way is
needed. Does this make sense?

I think I prefer my approach :

extract from ipynb diff

+     "cell_type": "markdown",
+     "metadata": {
+      "literate": {
+       "data": {
+        "a": "set([1, 2])",
+        "f": "<function <lambda> at 0x102383758>",
+        "hello": "'world'",
+        "iDontExist": "[ERROR] KeyError: u'iDontExist'",
+        "x": "16"
+       },

Don't you like it ?

No kidding, I think it would be great to extend to have

'variableName':{
  'text'  : 'whatever',
  'LaTeX' : '\w{ha}t_ever',
  'png'   : '24e5bcf6.....4'
}

@Carreau
Copy link
Member Author

Carreau commented Nov 28, 2012

To argue against me, :: is a JuliaLang operator, and as @JuliaLang might want to use notebook for web frontend, we might want their insight of wether this would be a problem.

@filmor
Copy link
Contributor

filmor commented Nov 28, 2012

How about extending the link syntax, in a similar way as it is for images, i.e. something like @[arguments](x) or

The value of x is @[arguments][id]

[id]: x

Or, to use it in LaTeX: $x = @[id] or $x = @(x)$

arguments could be something like live which would refresh the value whenever a cell is run.

Looks a bit clunky, but it would be quite flexible this way and I don't think that a prefix @ is used yet.

@Carreau
Copy link
Member Author

Carreau commented Dec 4, 2012

Does not merge cleanly anymore.

@Carreau
Copy link
Member Author

Carreau commented Dec 30, 2012

Other alternative I though of where things Jinja Style {{statement | options,with, commas}}, obviously bracket might not be great with LaTex , so one can considere variations like {{% | %}}, [[ | ]].
Also I don't think we want something to close to markdown, because it should not be parsed by markdown preprocessor.

This allow to add a name of variable in MD cell between colon like
::x:: and it will be replace by variable value after cell rendering.

value are store in metadata to stay between notebook save/reload
@Carreau
Copy link
Member Author

Carreau commented Dec 30, 2012

Rebased and update to support {{name | multiple options}} syntax. It is nicer because you can escape {{ as \{\{ which avoid the use of html sequences to display a bracket. And also, it does not conflict with latex as conversion process would replace variable before emitting the .tex file.

The only gotcha so far is that you can't actually write a string in a n-backtick environement in a markdown cell that would display as {{something}} with monospace font.
Assuming var=42, and (written -> display) then {{var}} -> 42 and {{var}} -> {{var}} so Ø -> {{var}} .

@Carreau
Copy link
Member Author

Carreau commented Jan 20, 2013

Any discussion feedback on the syntax we should use ?
Should I flag it as a dev feature not enable by default ?
Other thought ? Question ? Request ?

@ellisonbg
Copy link
Member

I will do a full review of this soon...

On Sun, Jan 20, 2013 at 11:00 AM, Bussonnier Matthias <
notifications@github.com> wrote:

Any discussion feedback on the syntax we should use ?
Should I flag it as a dev feature not enable by default ?
Other thought ? Question ? Request ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/2592#issuecomment-12475352.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@Carreau
Copy link
Member Author

Carreau commented Jan 21, 2013

Great ! Thanks.
I'll also think into incorporating it with the new nbviewer :-)

@minrk
Copy link
Member

minrk commented Jan 22, 2013

This is really important, and we need to think extremely carefully about how we expose this without mucking with markdown / mathjax.

I don't know if this is feasible, but I would actually love it if this could be implemented as a js plugin. Even if we don't go that way, understanding what would be involved in a third party developing something like this will help us understand what kind of capabilities the jsplugin arch needs to have.

@Carreau
Copy link
Member Author

Carreau commented Jan 22, 2013

I'm not sure this is totally doable as a js plugin as you indeed interfere with the way markdown is rendered.
You need to :

  • be warned when a cell is rendered. (easy, event)
  • ask kernel for value (doable)
  • ask the cell to be re-renderd (could do)
  • change the actual content of what would be rendered.
  • track which rendering request are "yours" and which one are not to avoid infinite loop.

What could be done is the ability to insert a call to a function between 'render request' call and 'do rendering'

The intermediate function would be responsible for calling 'do rendering' with the modified source.

The problem here is that if the kernel is busy, you wait for the cell to be rendered, and if you go the callback way it is difficult to chain the 'modifier' functions.

@minrk
Copy link
Member

minrk commented Jan 22, 2013

I don't mean to say that we must do it this way, I just think we should think it through, so we have it as a case study for either

A. something that our jsplugin arch should be able to handle, or
B. an example of something that we are deciding should be impossible via a js plugin.

The problem here is that if the kernel is busy, you wait for the cell to be rendered, and if you go the callback way it is difficult to chain the 'modifier' functions.

For instance, I think this would be an indicator that it has not been done correctly. The replacements should be async, and probably applied on the rendered HTML, rather than the markdown source forcing a re-render of the whole cell. So the steps would be:

  1. transform markdown to include placeholders
  2. submit requests for variables
  3. render markdown with placeholders
  4. when result arrives, rewrite the placeholders in rendered HTML, not markdown source.

@minrk
Copy link
Member

minrk commented Jan 22, 2013

It also suggests that our markdown render should be implemented as a series of prerender hooks (on markdown) and postrender hooks (on html), so that the transforms are more readily extensible. Current master has just the one of each - remove_math in prerender and replace_math in postrender.

@jasongrout
Copy link
Member

Just FYI, I've also been working a way to have live updates to a frontend as variables change. I've been working in the framework of interactive controls (see this example), but this could easily also work with rendered markdown things too.

@Carreau
Copy link
Member Author

Carreau commented Jan 22, 2013

For instance, I think this would be an indicator that it has not been done correctly. The replacements should be async,

Right now they are asyn, with placeholder while it waits.
IMHO it will just be brittle/difficult with plugin, even if there would be a way.

and probably applied on the rendered HTML, rather than the markdown source forcing a re-render of the whole cell.

You might not want that, as this would mean to 'break' the current structure of the HTML or guess the classes it would have been wrapped in when converted by markdown converter. It is possible to do that with mathjax as you know this is a full 'block'. And you will probably not be able to replace variable in math expression either like that.

Maybe you also want to restrict those variable to be purely text, in which case we can wrap them in a named span, (but not for mathjax) , but in the end, I would like to ask in the format to have a specific representation of a variable, which I'm not sure will play nicely with it.

So the steps would be: ...

We agree from 1 to 3.
I think 4 might be annoying in some cases, but I can try again.

So do we intend to :

  • allow not-text variable repr?
  • allow replacement in mathjax ?

How will it play with converters like nbconvert ?

The last question is how do you make the API ? Do you totally leave the post receive hook write the HTML ? Do you provide a 'placeholder/replacement value' interface ? How do you chain them ?

Either we impose those transformer to be synchronous

for fun in prerender_hook:
    input= fun(input)

Or fully async, we shoudl be able to do :

prerender_hook( cell_input , callback(transformed_input))

and By looping through all registered callback in reverse order build a nested loop of callback that in the end will call the native render.

@Carreau
Copy link
Member Author

Carreau commented Jan 22, 2013

Async chaining of methods... with a not so obvious closure.

var test1 = function( data , callback){
       callback(data+' test1')
}

var test2 = function( data , callback){
       callback(data+' test2')
}

var test3 = function( data , callback){
       callback(data+' test3')
}

var hooks= [test1,test2,test3]

var step = function(data,_){console.log('did receive :'+data)}
for (var i in hooks){
    // not obvious closure/IIFE
    step = (function(hook,step){return function(data){ hook(data,step)} })(hooks[i],step)
}

step('hi there !')

Prints

did receive :hi there ! test3 test2 test1

@ellisonbg
Copy link
Member

Your .docs/examples Notebook needs to be moved to /examples. Also, please make sure the style of the sample notebook matches the new formatting and naming of the example notebooks.

@ellisonbg
Copy link
Member

OK I finally got to playing with this. Some first impressions:

  • I like the {{var}} syntax better than the older ::var::.
  • I like that we are storing the literate data in the cell metadata so nbconvert can reconstruct things.
  • I love it!

I haven't done a line-by-line review of the code, but here are some broader comments:

  • I don't think we should even think about allowing arbitrary representations for this yet. That massively widens the scope of Markdown cells in a way that would have many unintended consequences. While I love this feature, I don't want us to diverge much from these being plain old markdown cells.
  • I think I am fine with the {{var}} syntax, but I agree with Min that we need to implement our markdown pre/post logic very carefully.
  • I don't think we should support the options syntax. It is overdesigning and would widen its scope too much.
  • I don't think this particular bit of code is well matched to being a plugin. I think we should decide if we want to have it as a feature in IPython and enable if for all users if we do.

@ellisonbg
Copy link
Member

One other comment, I think we should format the KeyError stuff slightly differently. To help set the error text apart from surrounding text, something like: [ERROR: KeyError: 'y']

@Carreau
Copy link
Member Author

Carreau commented Jan 25, 2013

I don't think we should even think about allowing arbitrary representations for this yet. That massively widens the scope of Markdown cells in a way that would have many unintended consequences. While I love this feature, I don't want us to diverge much from these being plain old markdown cells.

We can see later, let's focus on pure text then.

I think I am fine with the {{var}} syntax, but I agree with Min that we need to implement our markdown pre/post logic very carefully.

Ok, I'll look into that.

I don't think we should support the options syntax. It is overdesigning and would widen its scope too much.
I don't think this particular bit of code is well matched to being a plugin. I think we should decide if we want to have > it as a feature in IPython and enable if for all users if we do.

One other comment, I think we should format the KeyError stuff slightly differently. To help set the error text apart from surrounding text, something like: [ERROR: KeyError: 'y']

Protocol should not return an error as string.
It should return at least a dict with "status" and "value" as key. then you can "know" if there is an error or not.

@ellisonbg
Copy link
Member

This PR has been sitting for over a month now. This functionality is really important, but we want to get it right. Seems like you were going to work further on the code, in particular how the various stages of markdown processing are handled. Should we leave this open for you to continue that in place? Open an issue?

@asmeurer
Copy link
Contributor

I don't know how viable this is--you may end up having to do a bit of work to get it--but have you ever considered using an actual grammer and parser for Markdown? Then, extensions like this would be as simple as extending the grammar file, and you could be much more confident that the parser is still correct.

@Carreau
Copy link
Member Author

Carreau commented Feb 20, 2013

Right now, I'm overwhelmed.
So feel free to do what you wish.

As the right way to do it with parser or else, I have no idea.
The things is, everything we do on this client, we (or other) will have to replicate on nbconvert and other client, so we have to be really carefull.

@ellisonbg
Copy link
Member

OK here is what I think we should do: I think the design issues/questions are big enough that we really want to think through things before we take this any further. When things are a bit more calm, we should have a Google+ or HipChat session to work out the rest of the details. I am going to close this and open an issue to track the broader discussion.

@Carreau you know that we are super excited about this work - please don't take this as a vote against this idea. We will get these things figured out so you can move forward with this - also, when you have less on your plate. Thanks for you hard work!

@ellisonbg
Copy link
Member

Closing, discussion will continue in #2958.

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

Successfully merging this pull request may close these issues.

None yet

8 participants