Opened 12 years ago
Closed 12 years ago
#10362 closed defect (fixed)
Improve vector constructor documentation
Reported by: | Rob Beezer | Owned by: | jason, was |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6.1 |
Component: | linear algebra | Keywords: | |
Cc: | Jason Grout | Merged in: | sage-4.6.1.alpha3 |
Authors: | Rob Beezer | Reviewers: | Andrey Novoseltsev |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This is a documentation-only patch, other than some minor whitespace clean-up in the code. It brings the documentation of the vector()
constructor in line with the code. Specifically the handling of integer arguments (degree), the absence of entries (creating zero vectors), and some subtleties with dictionary input.
It suggests some code changes: negative integer arguments, more care with dictionary input, and a new zero_vector()
constructor, which will go on another ticket (see #10364).
Attachments (2)
Change History (8)
Changed 12 years ago by
Attachment: | trac_10362-vector-constructor-documentation.patch added |
---|
comment:1 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 12 years ago by
Status: | new → needs_review |
---|
comment:3 Changed 12 years ago by
Cc: | Jason Grout added |
---|
Changed 12 years ago by
Attachment: | trac_10362_reviewer.patch added |
---|
comment:4 Changed 12 years ago by
Milestone: | → sage-4.6.1 |
---|---|
Reviewers: | → Andrey Novoseltsev |
comment:5 Changed 12 years ago by
Status: | needs_review → positive_review |
---|
Andrey,
Thanks for the review and fixes. I'd noticed the "gluing" but had not experimented with a fix. I think I have a couple more tickets in-progress where I need to make that change as well.
I'm conflicted on the format of the OUTPUT block header and vacillate between the two options. Your change is fine.
Reviewer patch applies, tests, docbuilds just fine. So I'll flip this to positive review.
Thanks again, Rob
Release Manger
Apply original patch and then reviewer patch.
comment:6 Changed 12 years ago by
Merged in: | → sage-4.6.1.alpha3 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Looks like a nice improvement, except that dictionary-related parts seem to be weird, but this is addressed by another ticket.
In the original patch "INPUT" is "glued" to the numbered list above, so I indented the list to separate them. Also, I like when "OUTPUT" block looks the same as "INPUT" and in this particular case it really makes sense, I think, because the output description is so long. So I have added a blank line as well.
If you agree with the changes, please switch to positive review!