Opened 12 years ago

Last modified 4 weeks ago

#10077 needs_work enhancement

Add some info about sphinx and rest (including links) to developer guide

Reported by: jpflori Owned by: mvngu
Priority: trivial Milestone:
Component: documentation Keywords: documentation, coding convention
Cc: mvngu, jhpalmieri, kcrisman, ncohen Merged in:
Authors: Minh Van Nguyen Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

GitHub link to the corresponding issue

Description

There is a problem within http://www.sagemath.org/doc/developer/conventions.html. It says to use ".. NOTE::" and there is "NOTES:" in the example.

There is also nothing about "TESTS:" section which is often used whithin the source code.

The recurrent use of ".. blah ::", "::" and "sage: " could also be documented or at least directly point to the corresponding pages of Sphynx doc (even if looking at the examples is of course the best way to go and looking through Sphynx doc also explains many things).

Attachments (1)

trac-10077_clarify-conventions.patch (12.7 KB) - added by mvngu 12 years ago.

Download all attachments as: .zip

Change History (18)

Changed 12 years ago by mvngu

comment:1 Changed 12 years ago by mvngu

Authors: Minh Van Nguyen
Milestone: sage-4.6
Priority: minortrivial
Status: newneeds_review
Summary: Improve doc about coding conventions.Improve doc about coding conventions

There is a problem within http://www.sagemath.org/doc/developer/conventions.html. It says to use ".. NOTE::" and there is "NOTES:" in the example.

Fixed in the attached patch.

There is also nothing about "TESTS:" section which is often used whithin the source code.

Fixed in the attached patch.

The recurrent use of ".. blah ::", "::" and "sage: " could also be documented

Fixed in the attached patch.

or at least directly point to the corresponding pages of Sphynx doc

Fixed in the attached patch.

comment:2 Changed 12 years ago by novoselt

I a bit disagree with the interpretation of "TESTS" directive. I tend to put there any tests that do not really help in illustrating the function. For example, if there was a bug detected on some valid but long and perhaps obscure input, I think that it should go to TESTS. On the other hand, if some exceptions are not unusual, they may very well be illustrated in EXAMPLES, e.g. solving an overdetermined system of equations. Personally, I think that EXAMPLES are bits of code that show how to use a function. Bits of code that make sure that every branch of code got executed can be much more numerous than necessary for illustration.

What's the point of "notes" in lower case? Is it really necessary for something or maybe it is possible to list only .. NOTES::?

comment:3 Changed 12 years ago by jpflori

Thanks a lot for the patch. I guess with it the situation for newcomers as myself will be much clearer.

I kind of agree with novoselt statement about what EXAMPLES and TESTS blocks should be used for, but don't know what was decided by Sage devs because of the lack of doc before what you just produced.

About the "notes" and "warning" in lower case, they could maybe be mentionned but their use strongly discouraged (e.g. "you'll also find a lot of ".. notes::" in existing source code, which is valid, but the use of the lower case version is strongly discouraged", or "we are currently in the process of converting everything to uppercase").

comment:4 Changed 12 years ago by mhansen

Why are we saying that the uppercase version of the directives should be used? I can't think of any other Sphinx based documentation project that does that.

comment:5 in reply to:  2 Changed 12 years ago by mvngu

Replying to novoselt:

I a bit disagree with the interpretation of "TESTS" directive. I tend to put there any tests that do not really help in illustrating the function. For example, if there was a bug detected on some valid but long and perhaps obscure input, I think that it should go to TESTS.

And also providing the corresponding ticket number (if available) of the ticket for that bug.

On the other hand, if some exceptions are not unusual, they may very well be illustrated in EXAMPLES, e.g. solving an overdetermined system of equations.

This depends on the specific case. But I agree with this comment of yours.

Personally, I think that EXAMPLES are bits of code that show how to use a function.

Nod.

Bits of code that make sure that every branch of code got executed can be much more numerous than necessary for illustration.

Nod. Such code can go in the TESTS block. If there are too many such tests, create a sepatate test file, but don't include them in the reference manual.

What's the point of "notes" in lower case? Is it really necessary for something or maybe it is possible to list only .. NOTES::?

I tend to encourage the use of the Sphinx directive for notes. That is, use the lower-case ".. note::" or the upper-case version ".. NOTE::" (choose one). But Sphinx doesn't recognize ".. notes::" nor ".. NOTES::" (notice the "s" before the double colon). Both the Sphinx and the ReST documentation say that "notes" with an "s" is not supported.

See my comment at #10078 for reasons why I encourage (but not say "must") the use of upper-case versions of admonitions, etc.

comment:6 in reply to:  4 Changed 12 years ago by mvngu

Replying to mhansen:

Why are we saying that the uppercase version of the directives should be used? I can't think of any other Sphinx based documentation project that does that.

For consistency with "INPUT", "OUTPUT", "EXAMPLES" and "TESTS". See my comment at #10078. It's fine by me to say this: Sphinx supports both the lower- and upper-cases for "warning", "note", "math", etc.

comment:7 Changed 12 years ago by kcrisman

Cc: kcrisman added

comment:8 Changed 12 years ago by mariah

Status: needs_reviewneeds_work

patch trac-10077_clarify-conventions.patch fails to apply to sage-4.7.rc4. Perhaps needs to be rebased?

sage: hg_sage.apply("/home/mariah/trac-10077_clarify-conventions.patch")
cd "/home/mariah/sage/sage-4.7.rc4-x86_64-Linux-core2-fc-review-10077/devel/sage" && hg status
cd "/home/mariah/sage/sage-4.7.rc4-x86_64-Linux-core2-fc-review-10077/devel/sage" && hg status
cd "/home/mariah/sage/sage-4.7.rc4-x86_64-Linux-core2-fc-review-10077/devel/sage" && hg import   "/home/mariah/trac-10077_clarify-conventions.patch"
applying /home/mariah/trac-10077_clarify-conventions.patch
patching file doc/en/developer/conventions.rst
Hunk #2 FAILED at 109
1 out of 11 hunks FAILED -- saving rejects to file doc/en/developer/conventions.rst.rej
abort: patch failed to apply
sage:

comment:9 Changed 9 years ago by jdemeyer

Milestone: sage-5.11sage-5.12

comment:10 Changed 9 years ago by vbraun_spam

Milestone: sage-6.1sage-6.2

comment:11 Changed 9 years ago by vbraun_spam

Milestone: sage-6.2sage-6.3

comment:12 Changed 9 years ago by vbraun_spam

Milestone: sage-6.3sage-6.4

comment:13 Changed 8 years ago by kcrisman

Summary: Improve doc about coding conventionsAdd some info about sphinx and rest (including links) to developer guide

I have checked, and most of this is much better in the current developer guide. However, I think that a little more info about Sphinx and ReST as in this patch would be good, though perhaps not right at the start of that part!

comment:14 Changed 8 years ago by jhpalmieri

Cc: ncohen added

Nathann, since you've been looking at documentation recently, do you have any opinion about this?

comment:15 in reply to:  14 Changed 8 years ago by ncohen

Nathann, since you've been looking at documentation recently, do you have any opinion about this?

Well, as Karl-Dieter said Minh's point about Notes/Tests has been solved already. As for explaining the syntax of :: and sage: I do not know the doc sufficiently well to know where it should be done. I am not too worried about that, given that as Minh said people will learn that through looking at sources (and all examples from the manual that use this syntax already).

Minh is always right anyway.

Nathann

P.S.: If you have some time for a couple of reviews, I have two very short tickets that only move stuff around: #17614 and #17615

comment:16 Changed 6 weeks ago by mkoeppe

Milestone: sage-6.4

comment:17 Changed 4 weeks ago by chapoton

Cc: mvngu added; mvgnu removed
Note: See TracTickets for help on using tickets.