Opened 15 months ago

Closed 12 months ago

Last modified 6 months ago

#20690 closed enhancement (fixed)

Live documentation in Jupyter using Thebe

Reported by: nthiery Owned by:
Priority: major Milestone: sage-7.4
Component: documentation Keywords: days77, jupyter, thebe, notebook, sd75
Cc: vdelecroix, vbraun, rbeezer, slelievre, tmonteil Merged in:
Authors: Florent Cayré, Nicolas M. Thiéry Reviewers: Vincent Delecroix, Thierry Monteil
Report Upstream: N/A Work issues:
Branch: 8c8e731 (Commits) Commit:
Dependencies: #21309 Stopgaps:

Description (last modified by slelievre)

Thebe is a Jupyter javascript plugin for static sites that allows for rendering selected divs of the HTML as live cells connected to a Jupyter server:

https://oreillymedia.github.io/thebe/

The goal of this ticket is to use this technology to implement live documentation in the Jupyter notebook as we had in the legacy Sage notebook.

Kudos to Rob Beezer for pointing us to Thebe in his presentation of MathBookXML at Sage Days 77.

Steps:

  • [X] Configure Sphinx to add the Thebe javascript library in the static page
  • [X] Configure Sphinx to add a small header to our html page with:
    • [X] Inclusion of the Thebe javascript
    • [X] Thebe configuration: which divs to make live

Currently, we include all <pre> tags that contain the "sage:" prompt.

TODO: shall we change to <pre> tags that start with the "sage:" prompt?

TODO:

  • [X] Only activate Thebe if the page is served by a Jupyter instance

Currently we check that the protocol is http

  • [X] A button to activate live cells
  • [X] Configure the Jupyter notebook in Sage to somehow provide the server configuration to Thebe (not needed in fact)
  • [X] Preparse or customize/configure Thebe to support Sage's doctest syntax:
    • [X] Strip out the "sage: " prompts and "....:" and "... " continuation prompts
    • [X] Strip out the outputs

Bonus: show the included outputs below the cell until the new output is computed

What's been done here is: output is show until the "activate" button is clicked.

  • [X] Support doctests with several commands by spliting into several cells
  • [X] Ship Thebe from a dedicated spkg (#21309).

This ticket solves #17269. Follow-up ticket for possible improvements: #20893.

Change History (55)

comment:1 Changed 15 months ago by nthiery

  • Branch set to u/nthiery/live_documentation_in_jupyter_using_thebe

comment:2 Changed 15 months ago by nthiery

  • Branch changed from u/nthiery/live_documentation_in_jupyter_using_thebe to public/live_documentation_in_jupyter_using_thebe-20690

comment:3 Changed 15 months ago by nthiery

  • Commit set to 19f31f8567ac77094d2ee483e7adf9d6963a8dd7
  • Description modified (diff)
  • Summary changed from Live documentation in Jupyter using thebe to Live documentation in Jupyter using Thebe

New commits:

19f31f820690: added some examples of sage code in a fast-to-compile document. DONT MERGE IN SAGE

comment:4 Changed 15 months ago by nthiery

  • Branch changed from public/live_documentation_in_jupyter_using_thebe-20690 to public/live_documentation_in_jupyter_using_thebe-20690-experimental

comment:5 Changed 15 months ago by nthiery

  • Branch changed from public/live_documentation_in_jupyter_using_thebe-20690-experimental to public/live_documentation_in_jupyter_using_thebe-20690

The current branch is not to be merged in Sage, as it contains some edits in the Sage's faq that are just here for quick testing (compiling the faq is fast!).

comment:6 Changed 15 months ago by nthiery

  • Branch changed from public/live_documentation_in_jupyter_using_thebe-20690 to public/live_documentation_in_jupyter_using_thebe-20690-experimental

comment:7 Changed 15 months ago by nthiery

  • Description modified (diff)

comment:8 Changed 15 months ago by nthiery

  • Description modified (diff)

comment:9 Changed 15 months ago by git

  • Commit changed from 19f31f8567ac77094d2ee483e7adf9d6963a8dd7 to 5016cd83634381cd6be0f42154ea759744a7b272

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

5016cd820690: First implementation of a live documentation in Jupyter using Thebe

comment:10 Changed 15 months ago by nthiery

  • Description modified (diff)

comment:11 Changed 15 months ago by nthiery

  • Cc vdelecroix vbraun added

comment:12 Changed 15 months ago by nthiery

  • Cc rbeezer added
  • Description modified (diff)
  • Keywords days77 jupyter thebe notebook added

comment:13 Changed 15 months ago by nthiery

  • Description modified (diff)

comment:14 Changed 14 months ago by git

  • Commit changed from 5016cd83634381cd6be0f42154ea759744a7b272 to 07035fd4b6db42b55131c19026f274e4e624f33b

Branch pushed to git repo; I updated commit sha1. New commits:

32c9e6420690: Only show the activate button when there are cells to be activated
d6aaed220690: Disable the activate button once clicked
b5e95e420690: Factor out a thebe-sage setup function before complexifying it for upcoming functionalities
07035fd20690: On live doc activation, hide expected results and add a Run button before them

comment:15 Changed 14 months ago by vdelecroix

Everything seems to work (nice!). Some naive questions from a user point of view:

  • would it be possible to remove the "sage: " string in the cells? It would be convenient since it perturb the editor. It would even be worse with the continuation "....:" since these are not ignored by the sage preparser...

Less importantly:

  • why did you decide to have this "Run" and "Run Again" buttons? These do not exist in the standard Jupyter notebook. Would it be possible to simply remove them?
  • the cells are properly executed when pressing "Shift + Enter" but, contrarily to the case of the notebook, after pressing "Shift + Enter" the focus is kept in the same cell. Not sure that it is desirable to move the focus (since the next cell might be very far).

comment:16 Changed 14 months ago by vdelecroix

  • Description modified (diff)

comment:17 Changed 14 months ago by vdelecroix

  • Description modified (diff)
  • Reviewers set to Vincent Delecroix

comment:18 Changed 14 months ago by vdelecroix

I moved the part related to improvements to #20893. I definitely think that we should try to have something ready for the next beta release. And it is in good shape!

comment:19 Changed 14 months ago by fcayre

  • Branch changed from public/live_documentation_in_jupyter_using_thebe-20690-experimental to u/fcayre/thebe-20690
  • Commit changed from 07035fd4b6db42b55131c19026f274e4e624f33b to 5f9fc0e62bfe1c37f6a3419e9170ef0159f615f8
  • Description modified (diff)

comment:20 Changed 14 months ago by vdelecroix

  • Status changed from new to needs_review

As discussed with Florent (by mail) the first step should be ready (see #20893 for next steps). I am going through the changes and testing this version.

comment:21 follow-up: Changed 12 months ago by slelievre

  • Cc slelievre tmonteil added
  • Description modified (diff)

This ticket solves #17269.

comment:22 in reply to: ↑ 21 Changed 12 months ago by tmonteil

Replying to slelievre:

This ticket solves #17269.

Thanks for letting me know about the current ticket. This is indeed a blocker to leave sage notebook for good (this, and the fact that jupyter doen not handle .rst files as worksheets, let me continue to use the former during tutorials).

comment:23 follow-up: Changed 12 months ago by vdelecroix

  • Milestone changed from sage-7.3 to sage-7.4
  • Status changed from needs_review to needs_work

With the current version, the button activate does not appear in all situation. It does for the reference manual but not for Help -> Thematic Tutorials -> Tutorial: Sage Introductory Programming (PREP).

comment:24 Changed 12 months ago by tmonteil

  • Reviewers changed from Vincent Delecroix to Vincent Delecroix, Thierry Monteil

thebe.js should not be raw copied to Sage source code. Instead, it should be installed as a Sage package, so that it is easier to keep it up-to-date. Typically, the script you copied is adapted to Jupyter 4.0.5.

comment:25 follow-up: Changed 12 months ago by nthiery

Hi Thierry,

Thanks for looking at this!

It was Volker's suggestion to just raw copy thebe.js in the sources.

One part of the rationale is simplicity. The other is that, once jupyterlab will have matured out, thebe.js is likely to become a thin layer on top of it, if not just a part of it. So it's not that crucial to take the efforts of a super clean long term solution.

But I don't have a strong opinion either.

Last edited 12 months ago by nthiery (previous) (diff)

comment:26 in reply to: ↑ 25 Changed 12 months ago by tmonteil

Having a dedicated spkg is not such a huge effort, and there are tons of temporary things in Sage that wait to be cleaned for years, let us not make things worse when we can avoid it. Let me just give a try.

Last edited 12 months ago by tmonteil (previous) (diff)

comment:27 Changed 12 months ago by tmonteil

  • Branch changed from u/fcayre/thebe-20690 to u/tmonteil/thebe-20690

comment:28 follow-up: Changed 12 months ago by tmonteil

  • Commit changed from 5f9fc0e62bfe1c37f6a3419e9170ef0159f615f8 to ef6657d0dbd9c0008c5dae50bf18e47bee6f2e6c
  • Dependencies set to #21309
  • Description modified (diff)

I wrote #21309 to package thebe.js (needs review) and let the current branch depend on that one (i.e. i replaced the hardcoded code to a symlink to the file provided by the spkg). It works well on my machine.


New commits:

7d4fe86#21309 : package thebejs.
f508730Merge branch 'u/fcayre/thebe-20690' of trac.sagemath.org:sage into t/21309/package_thebe_js
ef6657d#20690 : use thebe.js from the thebejs package instead of the hardcoded one.

comment:29 in reply to: ↑ 23 Changed 12 months ago by tmonteil

  • Status changed from needs_work to needs_review

Replying to vdelecroix:

With the current version, the button activate does not appear in all situation. It does for the reference manual but not for Help -> Thematic Tutorials -> Tutorial: Sage Introductory Programming (PREP).

I double checked this. Actually, the button does not appear when the pages do not have cells (the first pages of the PREP are just pictures, no computations), but it does otherwise (see e.g. the "Calculus 1" page), so it is somehow a good feature.

comment:30 in reply to: ↑ 28 Changed 12 months ago by nthiery

Replying to tmonteil:

I wrote #21309 to package thebe.js (needs review) and let the current branch depend on that one (i.e. i replaced the hardcoded code to a symlink to the file provided by the spkg). It works well on my machine.

Ça c'est gentil!

For the record, I had Florent Cayré on the phone this morning. He started looking at this, but somehow he has a technical issue posting messages on trac which we did not manage to debug. Anyway, we should be able to review this shortly (e.g. tomorrow during the Sage Days).

comment:31 Changed 12 months ago by tmonteil

  • Keywords sd75 added

Great ! (i just arrived at Cernay, barbecue tonight!)

comment:32 Changed 12 months ago by nthiery

  • Branch changed from u/tmonteil/thebe-20690 to u/nthiery/thebe-20690

comment:33 Changed 12 months ago by git

  • Commit changed from ef6657d0dbd9c0008c5dae50bf18e47bee6f2e6c to 427875361ce7799bcce2b06641ba8624184b42c9

Branch pushed to git repo; I updated commit sha1. New commits:

427875320690: Added comment about Thebe and Sage's thebejs package

comment:34 Changed 12 months ago by slelievre

  • Description modified (diff)

(Just editing ticket description, fixing formatting.)

comment:35 follow-up: Changed 12 months ago by nthiery

Hi Vincent, Thierry, Besides merging the latest #21309 when it's polished, do you consider this as ready to go? It would be nice to be done with this! Thanks,

comment:36 Changed 12 months ago by tmonteil

  • Branch changed from u/nthiery/thebe-20690 to u/tmonteil/thebe-20690

comment:37 Changed 12 months ago by tmonteil

  • Commit changed from 427875361ce7799bcce2b06641ba8624184b42c9 to 11720b4c7f8bfdfb06f678c02a677754e7870553

I changed the symlink from local/share/thebejs/thebe.js to local/share/thebe/thebe.js according to the renaming done in #21309. Since the two branches were completely entangled (lot of merges), i went back to the last Florent's commit, fixed the symlink and added (and updated) Nicola's comment in thebe-sage.js header. Now the two branches are completely independant, apart from the very last merge.

If you plan to modify something in the current ticket or in #21309, please undo the last merge before, or it will start to knit again.


New commits:

e4768df#20690 : use thebe.js from the thebe package instead of the hardcoded one.
7199979#20690 : report updated version of Nicola's change in the disentangled branch.
520cdd0#21309 : thebejs -> thebe ; use hash in pkg name ; link to trac URL.
11720b4Merge branch 't/21309/package_thebe_js' into t/20690/thebe-20690

comment:38 in reply to: ↑ 35 Changed 12 months ago by tmonteil

Replying to nthiery:

Hi Vincent, Thierry, Besides merging the latest #21309 when it's polished, do you consider this as ready to go? It would be nice to be done with this! Thanks,

I did not inspect thebe-sage.js as i would have done for Python files. But its behaviour corresponds to the claimed feature and the code seem not malicious. So i guess it will be OK for me once the dependencies will be satisfied.

comment:39 Changed 12 months ago by nthiery

Thanks Thierry for the final version of #21309 (Jeroen is double checking it right now) and for cleaning the history.

I agree that we are on a thin line here since we don't have strong review standards set for js files like this one. For example we are missing tests! However writing such tests would take some infrastructure; we should discuss how to setup such infrastructure with the Jupyter people at some point, but for now I guess that's ok.

Vincent, any further comment?

Cheers,

Nicolas

comment:40 follow-up: Changed 12 months ago by tmonteil

  • Status changed from needs_review to needs_work

I have a doubt: to avoid race condition, i guess we should add thebe to the doc build dependencies.

comment:41 Changed 12 months ago by git

  • Commit changed from 11720b4c7f8bfdfb06f678c02a677754e7870553 to 22ae6378d820d7ad644c452c15ba335c27b1a6ab

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b8a7f32Merge branch 'develop' into t/20690/thebe-20690
22ae637#20690 : let the doc build depend on thebe.

comment:42 Changed 12 months ago by git

  • Commit changed from 22ae6378d820d7ad644c452c15ba335c27b1a6ab to 8c8e731a11790c135c906dbdf00dffd9fe616a9a

Branch pushed to git repo; I updated commit sha1. New commits:

7d4fe86#21309 : package thebejs.
28346f621309: added a brief line stating why Sage uses thebe
520cdd0#21309 : thebejs -> thebe ; use hash in pkg name ; link to trac URL.
8c8e731Merge branch 't/21309/package_thebe_js' into t/20690/thebe-20690

comment:43 in reply to: ↑ 40 ; follow-up: Changed 12 months ago by tmonteil

Replying to tmonteil:

I have a doubt: to avoid race condition, i guess we should add thebe to the doc build dependencies.

For this, i had to merge with sage.7.3 because the exact same line in build/make/deps changed there. Let us test the build like this (it might take some time). Again, if you plan to do some modifications, do not forget to undo the very last merge (with #21309) before.

comment:44 in reply to: ↑ 43 Changed 12 months ago by nthiery

Replying to tmonteil:

Replying to tmonteil:

I have a doubt: to avoid race condition, i guess we should add thebe to the doc build dependencies.

For this, i had to merge with sage.7.3 because the exact same line in build/make/deps changed there. Let us test the build like this (it might take some time).

Ok, thanks!

Again, if you plan to do some modifications, do not forget to undo the very last merge (with #21309) before.

We discussed this with Jeroen this morning. Unless Vincent has a specific requestion, we don't plan for further modifications.

comment:45 Changed 12 months ago by vdelecroix

(good for me)

comment:46 Changed 12 months ago by tmonteil

  • Status changed from needs_work to positive_review

comment:47 Changed 12 months ago by fbissey

I don't want to remove the positive review but I would like some info about /src/doc/common/themes/sage/static/thebe.js which reads

../../../../../../local/share/thebe/thebe.js

Would there be a better way to refer to the thebe.js file installed by the thebe package? The above assume a particular sage install. I don't usually let people get away with putting /local/ in anything these days as it is bad for stuff like sage-on-gentoo and more broadly sage-on-distro.

Last edited 12 months ago by fbissey (previous) (diff)

comment:48 Changed 12 months ago by tmonteil

I have no idea on how to do that. This kind of symlink already appears for mathjax, but the symlink is done when sagenb is installed. Unfortunately, there is no sage_documentation package where we could add a spkg-install script to install the symlink at build time. I you have an idea to improve this situation, it would be nice to implement it here and for mathjax as well.

comment:49 Changed 12 months ago by fbissey

I have no solution right now. I will have to find a way to live with it. I'll come up with something, I usually do in the end. It doesn't always make sense in vanilla sage unfortunately.

comment:50 Changed 12 months ago by vbraun

  • Branch changed from u/tmonteil/thebe-20690 to 8c8e731a11790c135c906dbdf00dffd9fe616a9a
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:51 Changed 11 months ago by jdemeyer

  • Commit 8c8e731a11790c135c906dbdf00dffd9fe616a9a deleted

Adding a symbolic link from the sources to some installation directory really makes no sense, especially because it is hardcoded. Having it installed by the thebe package would at least make slightly more sense. Of course, ideally we would just use thebe.js from the installation directory.

comment:52 Changed 11 months ago by jdemeyer

See #21527 for a follow-up on the symbolic link issue.

comment:53 Changed 10 months ago by tmonteil

Here is some feedback after real-life testing. I am currently teaching for a 3-week Sage tutorial, and there is a workflow issue, somehow thebe.js miss the main point of the sagenb live documentation.

On sagenb notebook, say the students work on some thematic tutorial, they do some tests, copy some lecturer's examples and want to keep their changes. What they can do is to click on File (top left) and then Copy worksheet so that the live documentation becomes a true sagenb worksheet they can save to come back on it later.

In the current case, we can do some tests and modifications (which is great), but all the work is lost when we close the webpage. We should have a button to export the current state into a (possibly degradated, we do not really care about the subtle html structure) .ipynb file that can then be saved.

comment:54 Changed 10 months ago by nthiery

Thanks Thierry for the feedback!

Yes, this is a very desirable feature, which we had already listed on #20893:

  • [ ] Add support in Thebe for basic export to Jupyter notebooks. A quality loss (in particular in terms of the hierarchical structure) is acceptable.

Do you mind merging this item with the new one you added there?

comment:55 Changed 6 months ago by jdemeyer

While preparing the report for OpenDreamKit?, I just noticed that this is completely broken: #22458

Note: See TracTickets for help on using tickets.