Opened 12 years ago
Last modified 5 weeks ago
#10080 needs_work enhancement
Fix documentation look in symbolic/expression.pyx
Reported by: | kcrisman | Owned by: | mvngu |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | documentation | Keywords: | sd35.5 |
Cc: | mvngu, jpflori | Merged in: | |
Authors: | Kenneth Smith, Kannappan Sampath | Reviewers: | Travis Scrimshaw, Kannappan Sampath |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
In this file, there are a lot of places where, in the html documentation, one sees TEST
areas that aren't highlighted, methods like simplify_log
referred to but not hyperlinked via :meth:
(in one case, somehow it is only meth:
by mistake), and so forth. I won't be more specific because it's throughout the file - you just have to look. For instance, in a number of places there is something like
TESTS:: sage: asf;dlkj
but since there isn't an intervening empty line this is typeset wrong.
Apply : trac_10080_docstringsfix1.patch, trac_10080_docstringsfix2.patch, attachment: trac_10080_docstringsfix3.patch]
Attachments (5)
Change History (27)
comment:1 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 11 years ago by
Authors: | → Kenneth Smith |
---|---|
Keywords: | sd35.5 added |
Status: | new → needs_review |
Changed 11 years ago by
Attachment: | trac_10080_symbolic_expression.patch added |
---|
comment:3 Changed 11 years ago by
Status: | needs_review → needs_work |
---|
comment:4 Changed 10 years ago by
The docstrings have been edited to confirm to our standards. (Personally, it helped me learn quite a bit about formatting the documentation and using an editor properly.)
comment:5 Changed 10 years ago by
Status: | needs_work → needs_review |
---|
comment:6 follow-up: 8 Changed 10 years ago by
Hey,
Many of the things that were there before were correct (see the conventions page). However the biggest thing to strive for (in a file) is consistency. For example, every occurrence of True
should be typeset as ``True``
and every trac reference should be of the form :trac:`10080`
. (Of course there are some exceptions to this "rule", but the reasoning should be clear and self-evident.) However this does clean up a fair amount and is a good first attempt.
Also, is your patch suppose to be the only patch applied or on top of Kenneth's patch?
Thanks,
Travis
comment:7 Changed 10 years ago by
Status: | needs_review → needs_work |
---|
comment:8 follow-up: 9 Changed 10 years ago by
Replying to tscrim: Hi!
Hey,
Many of the things that were there before were correct (see the conventions page). However the biggest thing to strive for (in a file) is consistency. For example, every occurrence of
True
should be typeset as``True``
I might be mistaken, but I went through them once. I would be glad to have some pointers for a start about the old style being the right one and "True".
and every trac reference should be of the form
:trac:`10080`
. (Of course there are some exceptions to this "rule", but the reasoning should be clear and self-evident.)
OK Will do!
However this does clean up a fair amount and is a good first attempt.
Also, is your patch suppose to be the only patch applied or on top of Kenneth's patch?
Mine is the supposed to be the only patch you apply.
Thanks,
Travis
comment:9 follow-up: 10 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Reviewers: | → Travis Scrimshaw |
Replying to knsam:
Replying to tscrim:
Many of the things that were there before were correct (see the conventions page). However the biggest thing to strive for (in a file) is consistency. For example, every occurrence of
True
should be typeset as``True``
I might be mistaken, but I went through them once. I would be glad to have some pointers for a start about the old style being the right one and "True".
For example, the INPUT:
and OUTPUT:
blocks should be:
INPUT: - ``arg_name`` -- Description of argument - ``second_arg`` -- Another description OUTPUT: Describing the output
Also, for multi-line doctests, it is just ...
, not ....:
(don't ask me why), I prefer EXAMPLES:
(with the s no matter if it is 1 or many), and capitalized .. SEEALSO::
and such blocks. A good check is to run sage -docbuild reference html
(after rebuilding sage via sage -b
) and look at the output in .../sage-branch/doc/output/html/...
in your favorite web-browser (Firefox had some issues last I knew, this might have been fixed...).
Mine is the supposed to be the only patch you apply.
For patchbot:
Apply only: trac_10080_docstrings_edit.patch
comment:10 follow-up: 11 Changed 10 years ago by
Replying to tscrim: Thank you for the specific pointers.
For example, the
INPUT:
andOUTPUT:
blocks should be:INPUT: - ``arg_name`` -- Description of argument - ``second_arg`` -- Another description OUTPUT: Describing the outputAlso, for multi-line doctests, it is just
...
, not....:
(don't ask me why), I preferEXAMPLES:
(with the s no matter if it is 1 or many), and capitalized.. SEEALSO::
and such blocks.
- OK, I think I understand what you mean about INPUT. Irrespective of the number of inputs, you have a bulleted list. Is this what we want? (Otherwise, I am not sure what is wrong with them.)
- Secondly, on the devel mailing list, I have seen that Keshav Kini was saying that it is better to have
....:
instead of...
(there is a also a ticket on trac: #10458). So, I am now lost.
- About, EXAMPLES, I'd go ahead and change. Capitalisation shall also be done.
A good check is to run
sage -docbuild reference html
(after rebuilding sage viasage -b
) and look at the output in.../sage-branch/doc/output/html/...
in your favorite web-browser (Firefox had some issues last I knew, this might have been fixed...).
Sure thing. Being a newbie that I am, my eyes are not trained enough to catch oddities. ;-)
Thank you for all the comments. I'll submit a patch to be applied on top of this. Thank you.
comment:11 Changed 10 years ago by
Replying to knsam:
- OK, I think I understand what you mean about INPUT. Irrespective of the number of inputs, you have a bulleted list. Is this what we want? (Otherwise, I am not sure what is wrong with them.)
That is correct.
- Secondly, on the devel mailing list, I have seen that Keshav Kini was saying that it is better to have
....:
instead of...
(there is a also a ticket on trac: #10458). So, I am now lost.
You'll notice that the ticket is not yet into sage (its status would be "closed" if it was), so those doctests will currently fail (you can run sage -t filename1 filename2 ...
from the terminal).
Thank you for all the comments. I'll submit a patch to be applied on top of this. Thank you.
Not a problem. You can just use the same patch (just make sure to check the "replace file if exists" during the attachments upload).
Best,
Travis
comment:12 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_work → needs_review |
Changed 10 years ago by
Attachment: | trac_10080_docstringsfix3.patch added |
---|
Not quite! Perhaps, the last!
comment:13 Changed 10 years ago by
Description: | modified (diff) |
---|
Hah! I have found one last glitch just now, which I do not how to fix:
x.__init__(...) initializes x; see help(type(x)) for signature
and
x.next() -> the next value, or raise StopIteration
which appear in the ExpressionIterator
class, towards the end of the built html.
I don't know how to fix this.
comment:14 Changed 10 years ago by
What are you trying to fix? By the way, the first patch is quite a bomb - I like the intent, but it could cause a lot of rebasing to be needed.
- Also,
:meth:bool
isn't right, becausebool
isn't a method, it's a function. - And in
taylor
you dedented stuff that should have remained indented; there are two notations supported, and they should be under that listing.
That said, these are mostly going to be useful changes! Did you look at the built documentation to check whether they "look right"? Travis has the right idea there.
comment:15 Changed 10 years ago by
I believe both are coming from the fact that ExpressionIterator
does not have any class-level docstring. Trying giving it something like
""" Iterator class for expressions. """
and see if that fixes the problems. Also once you integrate Karl-Dieter's changes, I'll give it another review (also, more of a side-note, I would prefer to have one patch since the other two patches are very small). Thanks.
comment:16 Changed 10 years ago by
Status: | needs_review → needs_work |
---|
comment:17 Changed 10 years ago by
Authors: | Kenneth Smith → Kenneth Smith, Kannappan Sampath |
---|---|
Reviewers: | Travis Scrimshaw → Travis Scrimshaw, Kannappan Sampath |
comment:18 Changed 9 years ago by
Milestone: | sage-5.11 → sage-5.12 |
---|
comment:19 Changed 9 years ago by
Milestone: | sage-6.1 → sage-6.2 |
---|
comment:20 Changed 9 years ago by
Milestone: | sage-6.2 → sage-6.3 |
---|
comment:21 Changed 8 years ago by
Milestone: | sage-6.3 → sage-6.4 |
---|
comment:22 Changed 5 weeks ago by
Milestone: | sage-6.4 |
---|
I made a patch that fixed all but two of the errors. I'm still getting:
docstring of sage.symbolic.expression:10: (ERROR/3) Unexpected indentation.
and
docstring of sage.symbolic.expression:12: (WARNING/2) Block quote ends without a blank line; unexpected unindent.