Opened 5 years ago
Closed 5 years ago
#17592 closed enhancement (fixed)
rephrase "LaTeX formating" and "Writing doctests"
Reported by:  ncohen  Owned by:  

Priority:  major  Milestone:  sage6.5 
Component:  documentation  Keywords:  
Cc:  kcrisman, vdelecroix, tmonteil  Merged in:  
Authors:  Nathann Cohen  Reviewers:  John Palmieri 
Report Upstream:  N/A  Work issues:  
Branch:  dba4110 (Commits)  Commit:  dba41109f7fbaef35968be531494e0c387668f30 
Dependencies:  #17589  Stopgaps: 
Description
With this, the explanations about latex formatting are a bit clearer, and we finally have a section that says what doctests should test. I may not have thought of everything.
Nathann
Change History (26)
comment:1 Changed 5 years ago by
 Branch set to public/17592
 Commit set to 216eb972fcb5c224243db440fa0a43c1ec85498c
 Status changed from new to needs_review
comment:2 followup: ↓ 3 Changed 5 years ago by
Re your change in the LaTeX Typesetting
section from
In ReST documentation, you use backticks \` to mark LaTeX code to be typeset. In Sage docstrings, you may also use dollar signs instead.
to
In ReST documentation LaTeX code is allowed, and is marked with **backticks or dollar signs**:
This is not entirely accurate: in ReST documentation, you can't use dollar signs; those are only available in Sage's documentation. Maybe say "In Sage's ReST documentation ..."?
I would also point out that $$math expression$$
is oldfashioned and deprecated in LaTeX; \[math expression]\]$
is much preferred.
Why remove the instructions about :nowrap:
? Is the warning about it no longer accurate?
comment:3 in reply to: ↑ 2 Changed 5 years ago by
Hello John !
First, let me say that I have absolutely nothing against seeing this patch reviewed very soon, but it has a dependency that must be reviewed first otherwise we are sure to have conflict between the two of them. So if you have some time to review a doc ticket, could you start with #17589 ? It is very short !
This is not entirely accurate: in ReST documentation, you can't use dollar signs; those are only available in Sage's documentation. Maybe say "In Sage's ReST documentation ..."?
I see. We may also not mention those dollar signs as we try always use backticks anyway nowadays ?...
I would also point out that
$$math expression$$
is oldfashioned and deprecated in LaTeX;\[math expression]\]$
is much preferred.
I never used that, but I can make the change. To me all about LaTeX is oldfashioned and deprecated. This thing cannot detect a syntax error properly.
Why remove the instructions about
:nowrap:
? Is the warning about it no longer accurate?
It just does not seem to work. I first wanted to add an illustration of this nowrap as I did for the examples above, and it had no effect. Then I looked for ":nowrap:" in Sage's source code and found out that it was used only once.... and that it had no effect either.
Nathann
comment:4 Changed 5 years ago by
 Commit changed from 216eb972fcb5c224243db440fa0a43c1ec85498c to 467d02db83994ac80d4edfebfbece9c9aae2bcdc
Branch pushed to git repo; I updated commit sha1. New commits:
467d02d  trac #17592: Review

comment:5 Changed 5 years ago by
If I make the change

src/sage/graphs/graph_decompositions/vertex_separation.pyx
diff git a/src/sage/graphs/graph_decompositions/vertex_separation.pyx b/src/sage/graphs/graph_dec index d1efc0e..0ac8a56 100644
a b sets such that an ordering `v_1, ..., v_n` of the vertices correspond to 146 146 **MILP formulation:** 147 147 148 148 .. MATH:: 149 :nowrap:150 151 149 \begin{alignat}{2} 152 150 \text{Minimize:} 153 151 &z&\\
then make docpdf
fails for me. So I think :nowrap:
is important for the PDF documentation.
comment:6 Changed 5 years ago by
Same result on my computer...
How should it be documented, though ? "Makes no difference in the html doc but one example of our doc would not compile otherwise" ? :/
Nathann
comment:7 Changed 5 years ago by
 Commit changed from 467d02db83994ac80d4edfebfbece9c9aae2bcdc to 84a8b233a6602a644eb63541d4846aa832e2d22b
Branch pushed to git repo; I updated commit sha1. New commits:
84a8b23  trac #17592: :nowrap: again

comment:8 Changed 5 years ago by
Here it is ! I made some further attempts at understanding the behaviour: no difference at all for html [1], but the pdf compiles.
Nathann
[1] in particular the example currently in the manual resuts in a "This is now plain text so I can do things like" written in math mode
comment:9 Changed 5 years ago by
I propose the following wording:

src/doc/en/developer/coding_basics.rst
diff git a/src/doc/en/developer/coding_basics.rst b/src/doc/en/developer/coding_basics.rst index abeb127..298af06 100644
a b the following are valid:: 517 517 Return $\sin(x)$. 518 518 """ 519 519 520 **MATH block:** It is similar to LaTeX'syntax ``\[<math expression>\]`` (or520 **MATH block:** This is similar to the LaTeX syntax ``\[<math expression>\]`` (or 521 521 ``$$<math expression>$$``). For instance:: 522 522 523 523 .. MATH:: … … The **aligned** environment works as it does in LaTeX:: 548 548 g(x) & = x^x  f(x  2) 549 549 \end{aligned} 550 550 551 For **nonmath** LaTeX environments (like ``align``), the pdf documentation 552 will not compile unless you add a **:nowrap:** flag to the MATH mode:: 551 When building the PDF documentation, everything is translated to LaTeX 552 and each MATH block is automatically wrapped in a math environment  553 in particular, it is turned into ``\begin{gather} expression 554 \end{gather}``. So if you want to use a LaTeX environment (like 555 ``align``) which in ordinary LaTeX would not be wrapped like this, you 556 must add a **:nowrap:** flag to the MATH mode:: 553 557 554 558 .. MATH:: 555 559 :nowrap:
I can commit this to the branch if you want, or I can try to review more and see if there are other changes I want to make first, so we don't have as many commits.
comment:10 Changed 5 years ago by
Hello John !
No problem for your first modification. As for the second, I had dropped on purpose the explanation about 'wrapping' as it made no sense that it applied only to pdf and not to html. ALso, I did not want to go in such depth about something which is very very very marginally useful to us (one instance in all of Sage's code!) and really seems to be a bug more than anything else. What would you think of only keeping your second sentence ? Something like:
+ When building the PDF documentation, a LaTeX environment
+ which should not appear in math mode (e.g. ``align``) may
+ become a problem. In order to use them, you will have to
+ add a `:nowrap:` flag::
While I prefer a short sentence like that, I will be okay with whatever you decide on this respect. My only aim is to make it easy to read and to use, and I often worry about not dragging people into reading things that are of no interest to them (which explains the bold words I add everywhere).
If you can take the time to do a full review of this branch this will probably make my life easier; fixing everything at once is much less work.
Thanks,
Nathann
comment:11 followup: ↓ 12 Changed 5 years ago by
I think I would prefer to leave more detail: some users will be familiar with LaTeX and will want to know what's going on, and some will wonder about the "nowrap" name. An extra sentence seems okay in this case, but several paragraphs about it would definitely be overkill.
comment:12 in reply to: ↑ 11 Changed 5 years ago by
I think I would prefer to leave more detail: some users will be familiar with LaTeX and will want to know what's going on, and some will wonder about the "nowrap" name. An extra sentence seems okay in this case, but several paragraphs about it would definitely be overkill.
Okay. We can also add a link toward the original Sphinx documentation if you believe that it might interest some readers: http://sphinxdoc.org/latest/ext/math.html?highlight=nowrap#directivemath
Nathann
comment:13 followup: ↓ 15 Changed 5 years ago by
I'm in the middle of reviewing this. You say "Random of systematic tests". Should I change "of" to "or", delete "of" altogether, or make some other change? What did you intend?
comment:14 Changed 5 years ago by
 Commit changed from 84a8b233a6602a644eb63541d4846aa832e2d22b to e40fe19cf613d6f64072a487371a933095b5a0da
Branch pushed to git repo; I updated commit sha1. New commits:
e40fe19  trac #17592: Typo/Rephrase

comment:15 in reply to: ↑ 13 Changed 5 years ago by
Hello !
I'm in the middle of reviewing this. You say "Random of systematic tests". Should I change "of" to "or", delete "of" altogether, or make some other change? What did you intend?
It was a 'or', but sentence was a bit unclear so I rephrased it.
We are in the worst timezones to work on something it seems ^^;
Nathann
comment:16 followup: ↓ 18 Changed 5 years ago by
Here are some minor changes, mostly rewordings. The centered text was different from the style in the rest of the document, so I removed it.
comment:17 Changed 5 years ago by
 Commit changed from e40fe19cf613d6f64072a487371a933095b5a0da to fb4a68048d5acd303154311b6c58027aa63bf962
Branch pushed to git repo; I updated commit sha1. New commits:
fb4a680  #17592: referee fixes

comment:18 in reply to: ↑ 16 Changed 5 years ago by
Here are some minor changes, mostly rewordings.
Okay.
The centered text was different from the style in the rest of the document, so I removed it.
HMmm... Well, that was the only way I found to split the lists, as there is already a lot of bold text in there.. Hmmm....
Well, I really prefer it centered but... Well, perhaps it is not so bad :/
Nathann
comment:19 Changed 5 years ago by
Is there anything else that you want to change in this patch, or can I switch it to positive review
?
Nathann
comment:20 Changed 5 years ago by
 Reviewers set to John Palmieri
 Status changed from needs_review to positive_review
I think it's ready.
comment:21 Changed 5 years ago by
Thanks !
Nathann
comment:22 Changed 5 years ago by
Conflicts, probably with #17589
comment:23 Changed 5 years ago by
 Status changed from positive_review to needs_work
comment:24 Changed 5 years ago by
 Commit changed from fb4a68048d5acd303154311b6c58027aa63bf962 to dba41109f7fbaef35968be531494e0c387668f30
comment:25 Changed 5 years ago by
 Status changed from needs_work to positive_review
comment:26 Changed 5 years ago by
 Branch changed from public/17592 to dba41109f7fbaef35968be531494e0c387668f30
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
trac #17555: Remove the dev scripts' documentation
trac #17555: Some other references to the doc
trac #17555: Merged with #17534
trac #17555: remove mention of the dev scripts
trac #17555: Merged with 6.5.beta5
trac #17589: Small changes in the developer's manual table of contents
trac #17592: rephrase "LaTeX formating" and "Writing doctests"