#15377 closed defect (fixed)
improve documentation for projective normalize_coordinates
Reported by: | bhutz | Owned by: | bhutz |
---|---|---|---|
Priority: | trivial | Milestone: | sage-5.13 |
Component: | algebraic geometry | Keywords: | sage-days55, days54 |
Cc: | Merged in: | sage-5.13.beta3 | |
Authors: | Ben Hutz | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
For some base rings (such as polynomial rings over a field), the output of the function may seem counter intuitive even though the gcd is correct. The documentation is inadequate to describe this situation.
Attachments (1)
Change History (11)
comment:1 Changed 4 years ago by
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
One thing for line 394; instead of "Beware" (or "Be aware"), I think this would be best formatted as
.. WARNING:: The gcd depends on the base ring.
After this it will be a positive review.
Edit - one other thing, could you remove the comma ,
from line 449.
Best,
Travis
comment:3 Changed 4 years ago by
- Owner changed from (none) to bhutz
comment:4 Changed 4 years ago by
- Status changed from needs_review to positive_review
It fixed the typo and clarified the functionality.
Examples do what they should.
doctest looks good.
-Adam
comment:5 Changed 4 years ago by
- Status changed from positive_review to needs_work
I missed this the first go-around but you need to have a proper commit message.
Also why did you choose not to use the .. WARNING::
for the gcd dependency on the base ring?
comment:6 Changed 4 years ago by
opps. I forgot the commit message. The warning field is a good suggestion and has now been implemented for this patch.
comment:7 Changed 4 years ago by
Last thing; I noticed (I edited my first comment) is that the comma here
A polynomial ring over a ring, gives the more intuitive result.
should be removed. Thanks!
comment:8 Changed 4 years ago by
- Keywords days54 added
- Reviewers set to Travis Scrimshaw
- Status changed from needs_work to positive_review
Positive review. Thanks.
comment:9 Changed 4 years ago by
- Merged in set to sage-5.13.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
added doumentation and examples. Cleaned up formatting of examples.