HTML documentation: Show preparsed doctests using inline tabs#37083
HTML documentation: Show preparsed doctests using inline tabs#37083vbraun merged 15 commits intosagemath:developfrom
Conversation
…) using inline tabs
…E_PREPARSED_DOC (default yes)
…-preparsed-examples
| echo "--------------------------------------------------------------------8<-----------------------------" | ||
| echo "::endgroup::" | ||
| echo "Failure applying $PULL_URL as a patch, resetting" | ||
| echo "Failure applying $PULL_SHORT as a patch, resetting" |
There was a problem hiding this comment.
I like the full URL since it easy to open the corresponding PR. Either way, these changes are unrelated to the actual goal of the PR and thus should be moved to somewhere else.
There was a problem hiding this comment.
The full URL appears just above. The improvement is that the repeated one is shortened so that there is not a second, duplicate link.
I'll not open a separate PR for this.
There was a problem hiding this comment.
Why not? This is clearly not connected to main theme of this PR.
There was a problem hiding this comment.
You've just finished the review of this one line change, no?
There was a problem hiding this comment.
No, and there are also more changes here that seem unrelated.
There was a problem hiding this comment.
You commented, I responded to your comment. What's missing for the review of this one line change.
There was a problem hiding this comment.
The rules of review, spelled out for you:
- Provide the necessary information.
- Do not use vagueness as a tool.
- Do not ask for information that has already been provided.
| @@ -0,0 +1 @@ | |||
| standard | |||
There was a problem hiding this comment.
Adding a new standard package needs a period where it is optional and then a vote on the mailing list first, right?
There was a problem hiding this comment.
We don't do that for trivialities.
| standard.add_argument("--no-plot", dest="no_plot", | ||
| action="store_true", | ||
| help="do not include graphics auto-generated using the '.. plot' markup") | ||
| standard.add_argument("--no-preparsed-examples", dest="no_preparsed_examples", |
There was a problem hiding this comment.
What's the use case for this option?
There was a problem hiding this comment.
It's faster and some people may not want to see the tabs.
There was a problem hiding this comment.
Speedwise it doesn't seem to make a change (by looking at the github workflow execution time). Thus, to reduce complexity I prefer to just remove the option here and always build the docs with the parsed python tab.
There was a problem hiding this comment.
Noted that this is your preference. I won't remove it.
There was a problem hiding this comment.
Why do you want to make this feature configurable?
There was a problem hiding this comment.
More rules for review
- Check the facts about an unfamiliar domain before making claims about it
Specifically here: Any developer who has worked with the Sage docbuild system knows that we have long had a range of options that configure global aspects of it. There's SAGE_SKIP_PLOT_DIRECTIVE (which you see even in the diff of the present PR...), SAGE_DOC_UNDERSCORE, SAGE_USE_CDNS, and we used to have SAGE_DOC_MATHJAX, SAGE_DOC_JSMATH until recently.
There was a problem hiding this comment.
In each of these cases there is a good reason for why the switch exists beyond "some people may not want to" have xyz.
There was a problem hiding this comment.
Just a moment ago you denied that the switches even exist.
Now you say they exist but they have a better reason to exist.
This is called "moving the goalposts", a well-known standard technique.
There was a problem hiding this comment.
The goalpost stayed the same: we should not add config options to toggle default documentation features off just because "some people may not want to see [xyz]".
There was a problem hiding this comment.
Yes, you're repeating yourself.
Just because you have this opinion does not mean that this PR needs work.
|
There are overlapped tabs like
This is from Sage can handle very large numbers, even numbers with millions or billions of
digits.
::
sage: factorial(100)
93326215443944152681699238856266700490715968264381621468592963895217599993229915608941463976156518286253697920827223758251185210916864000000000000000000000000
::
sage: n = factorial(1000000) # about 1 second
sage: len(n.digits())
5565709
This computes at least 100 digits of :math:`\pi`. It would be better to separate tabs for the two blocks of examples. Is this possible? |
|
Thanks for finding this bug. I'll investigate |
|
I tested with live documentation. Very nice. One suggestion. How about "Live" instead of "Sage (live)"? Short and easy to read, and without parentheses. Sample view:
|
|
I think I prefer "Sage (live)" for its clearer semantics ("same language (but a variant)") and extensibility (we can add "Python (live)" later). But I also do have to admit that I generally like parentheses, which is probably related to the amount of time I spent with Lisp in the past~ |
OK. This is just a matter of personal preference, where the author's preference takes precedence.
That is interesting, though I cannot imagine it now.
I don't like parentheses, as they add branches (obstructions) to the (linear) flow of sentences. I think I learned it from Knuth's advices on writing: one of the last things he does after writing a sentence is to remove unnecessary parentheses from it. |
SageMath version 10.3.beta8, Release Date: 2024-02-13
|
How to turn off tabs? Is it possible to turn off tabs with It seems that turns on tabs. |
Yes, that's right. Would you prefer the tabless version for this case generally, or as an option? |
|
I am not sure... What is the use of the preparsed doc? I guess that primarily advanced users of distribution packages may find it useful. I can imagine that a beginner may become aware of the preparsing mechanism and see how sage syntax is different from python syntax, which is an essential knowledge for effective use of sage. Users in the middle ground may find tabs (with preparsed doc) just take spaces for information of little use. Having tabs for each example that synchronize anyway seems a waste of space. I feel that somehow we should have configuration option or switch on pages to support the users in the middle ground. And whatever option or switch we implement should get along with the configuration option and switch for live doc. I am not sure how we could do this without making things too complicated... |
Yes, all of that. But it also helps anyone who is already an experienced Python programmer and comes across Sage. It makes it obvious and immediately testable that Sage is actual Python and usable from Python as a library, not some kind of quasi-Python, Python-like, Python-inspired kind of specialty language, or Python with training wheels.
Yes, the vertical space is precious, which is exactly why I made it disablable.
A persistent switch (is the light/dark selector persistent? using cookies?) for users who only want to see one version and save the vertical space could be an interesting follow-up. But that needs to be done by someone other than me; I stopped following web technologies around 1997;) |
|
I think I can suggest these for now:
|
Nowadays, AI helps humans catch up :-) |
Why would that possibly be an improvement? |
Why would that be better? I much prefer to click the middle tab instead of having to chase down that shy "Make live" button that lives in hiding near the window frame |
I assumed all tabs are already in either "parsed" or "preparsed" by default as the user want. Then the user would want to switch to the other version temporarily for a particular tab. But right... in my proposal, it is not handy for the user to choose the the default, unless to make a full doc build. OK. I am not strong about this. |
By having the two features orthogonal, we don't need to add an additional tab for "live preparsed". But I agree that the button is not so visible... |
|
By the way, live doc seems broken (as seen in the doc preview). Perhaps temporarily. Thebe was upgraded to 0.9.0 recently. |
Are we not pinning the versions of the components there? |
|
We are using https://unpkg.com/thebe@latest/lib/index.js via patched jupyter-sphinx. |
|
Fixing in #37637 |
|
You may want these refinements: diff --git a/src/doc/common/static/custom-jupyter-sphinx.css b/src/doc/common/static/custom-jupyter-sphinx.css
index a68a5cb05aa..e79b47b4539 100644
--- a/src/doc/common/static/custom-jupyter-sphinx.css
+++ b/src/doc/common/static/custom-jupyter-sphinx.css
@@ -1,5 +1,5 @@
div.jupyter_container {
- margin: .5rem 0;
+ border: 0;
}
div.jupyter_container + div.jupyter_container {
diff --git a/src/sage_docbuild/conf.py b/src/sage_docbuild/conf.py
index 145105bb814..1c705c68d1f 100644
--- a/src/sage_docbuild/conf.py
+++ b/src/sage_docbuild/conf.py
@@ -903,7 +903,7 @@ class SagecodeTransform(SphinxTransform):
linenostart=1)
cell_node += cell_input
container = TabContainer("", type="tab", new_set=False)
- textnodes = [Text('Sage (live)')]
+ textnodes = [Text('Sage Live')]
label = Label("", "", *textnodes)
container += label
content = Container("", is_div=True, classes=["tab-content"])"Sage Live" or "Sage live" (no parentheses please). (before) (see the double border) |
|
Thank you, committed as dea37e7. |
|
Documentation preview for this PR (built with commit dea37e7; changes) is ready! 🎉 |
Every doctest block is decorated with tabs, by default showing the original doctest in Sage syntax, and offering a preparsed, pure Python version of it. The tabs are synchronized across the page.
If @kwankyu's live documentation is being built, it is offered as another tab Sage (live). When this tab is selected, it automatically starts the Thebe/Binder; it is not necessary to find and push the "Make live" button.
Preview
Fixes #35791
📝 Checklist
⌛ Dependencies
.. envvar::to define environment variables #37065