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

rephrase "LaTeX formating" and "Writing doctests" #17592

Closed
nathanncohen mannequin opened this issue Jan 6, 2015 · 32 comments
Closed

rephrase "LaTeX formating" and "Writing doctests" #17592

nathanncohen mannequin opened this issue Jan 6, 2015 · 32 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 6, 2015

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

Depends on #17589

CC: @kcrisman @videlec @sagetrac-tmonteil

Component: documentation

Author: Nathann Cohen

Branch/Commit: dba4110

Reviewer: John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/17592

@nathanncohen nathanncohen mannequin added this to the sage-6.5 milestone Jan 6, 2015
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 6, 2015

Branch: public/17592

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 6, 2015

New commits:

49ab0bftrac #17555: Remove the dev scripts' documentation
40220f7trac #17555: Some other references to the doc
fdc52e9trac #17555: Merged with #17534
5c37a78trac #17555: remove mention of the dev scripts
805ea9atrac #17555: Merged with 6.5.beta5
5212473trac #17589: Small changes in the developer's manual table of contents
216eb97trac #17592: rephrase "LaTeX formating" and "Writing doctests"

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 6, 2015

Commit: 216eb97

@nathanncohen nathanncohen mannequin added the s: needs review label Jan 6, 2015
@jhpalmieri
Copy link
Member

comment:2

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 old-fashioned and deprecated in LaTeX; \[math expression]\]$ is much preferred.

Why remove the instructions about :nowrap:? Is the warning about it no longer accurate?

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 6, 2015

comment:3

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 old-fashioned 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 old-fashioned 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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 6, 2015

Changed commit from 216eb97 to 467d02d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 6, 2015

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

467d02dtrac #17592: Review

@jhpalmieri
Copy link
Member

comment:5

If I make the change

diff --git a/src/sage/graphs/graph_decompositions/vertex_separation.pyx b/src/sage/graphs/graph_dec
index d1efc0e..0ac8a56 100644
--- a/src/sage/graphs/graph_decompositions/vertex_separation.pyx
+++ b/src/sage/graphs/graph_decompositions/vertex_separation.pyx
@@ -146,8 +146,6 @@ sets such that an ordering `v_1, ..., v_n` of the vertices correspond to
 **MILP formulation:**
 
 .. MATH::
-    :nowrap:
-
     \begin{alignat}{2}
     \text{Minimize:}
     &z&\\

then make doc-pdf fails for me. So I think :nowrap: is important for the PDF documentation.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 7, 2015

comment:6

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 7, 2015

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

84a8b23trac #17592: :nowrap: again

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 7, 2015

Changed commit from 467d02d to 84a8b23

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 7, 2015

comment:8

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

@jhpalmieri
Copy link
Member

comment:9

I propose the following wording:

diff --git a/src/doc/en/developer/coding_basics.rst b/src/doc/en/developer/coding_basics.rst
index abeb127..298af06 100644
--- a/src/doc/en/developer/coding_basics.rst
+++ b/src/doc/en/developer/coding_basics.rst
@@ -517,7 +517,7 @@ the following are valid::
         Return $\sin(x)$.
         """
 
-**MATH block:** It is similar to LaTeX' syntax ``\[<math expression>\]`` (or
+**MATH block:** This is similar to the LaTeX syntax ``\[<math expression>\]`` (or
 ``$$<math expression>$$``). For instance::
 
     .. MATH::
@@ -548,8 +548,12 @@ The **aligned** environment works as it does in LaTeX::
      g(x) & = x^x - f(x - 2)
     \end{aligned}
 
-For **non-math** LaTeX environments (like ``align``), the pdf documentation
-will not compile unless you add a **:nowrap:** flag to the MATH mode::
+When building the PDF documentation, everything is translated to LaTeX
+and each MATH block is automatically wrapped in a math environment --
+in particular, it is turned into ``\begin{gather} expression
+\end{gather}``.  So if you want to use a LaTeX environment (like
+``align``) which in ordinary LaTeX would not be wrapped like this, you
+must add a **:nowrap:** flag to the MATH mode::
 
     .. MATH::
        :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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 7, 2015

comment:10

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

@jhpalmieri
Copy link
Member

comment:11

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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 7, 2015

comment:12

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://sphinx-doc.org/latest/ext/math.html?highlight=nowrap#directive-math

Nathann

@jhpalmieri
Copy link
Member

comment:13

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?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 10, 2015

Changed commit from 84a8b23 to e40fe19

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 10, 2015

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

e40fe19trac #17592: Typo/Rephrase

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 10, 2015

comment:15

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

@jhpalmieri
Copy link
Member

comment:16

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 10, 2015

Changed commit from e40fe19 to fb4a680

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 10, 2015

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

fb4a680#17592: referee fixes

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 10, 2015

comment:18

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

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 10, 2015

comment:19

Is there anything else that you want to change in this patch, or can I switch it to positive review ?

Nathann

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:20

I think it's ready.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 10, 2015

comment:21

Thanks !

Nathann

@vbraun
Copy link
Member

vbraun commented Jan 12, 2015

comment:22

Conflicts, probably with #17589

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 12, 2015

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

6a2a419#17589: trivial fixes
dba4110trac #17592: Merged with updated #17589

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 12, 2015

Changed commit from fb4a680 to dba4110

@vbraun
Copy link
Member

vbraun commented Jan 13, 2015

Changed branch from public/17592 to dba4110

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

No branches or pull requests

2 participants