Opened 9 years ago
Closed 9 years ago
#12767 closed defect (fixed)
Clean up dead links in sage/rings/number_field documentation
Reported by: | davidloeffler | Owned by: | davidloeffler |
---|---|---|---|
Priority: | major | Milestone: | sage-5.1 |
Component: | number fields | Keywords: | sd40.5 |
Cc: | AlexGhitza | Merged in: | sage-5.1.beta2 |
Authors: | David Loeffler | Reviewers: | Karl-Dieter Crisman, William Stein, Benjamin Jones |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Touching all files in sage/rings/number_field and running sage -docbuild --warn-links reference html
reveals a bunch of errors:
<autodoc>:1: WARNING: py:class reference target not found: sage.categories.map.Map <autodoc>:1: WARNING: py:class reference target not found: sage.categories.map.Map /storage/masiao/sage-5.0.beta10/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.py:docstring of sage.rings.number_field.number_field.NumberField_generic.reduced_gram_matrix:13: WARNING: py:meth reference target not found: Minkowski_embedding docstring of sage.rings.number_field.number_field_base.NumberField:5: WARNING: py:class reference target not found: NoetherianRing /storage/masiao/sage-5.0.beta10/local/lib/python2.7/site-packages/sage/rings/number_field/number_field_ideal.py:docstring of sage.rings.number_field.number_field_ideal.NumberFieldIdeal.S_ideal_class_log:1: WARNING: py:meth reference target not found: _ideal_class_log <autodoc>:1: WARNING: py:class reference target not found: sage.categories.map.Map
The patch below fixes these. It also sorts out a genuine bug I spotted in the process (the maximal_order
method of relative number fields doesn't have an option to return a p-maximal order).
Attachments (2)
Change History (12)
comment:1 Changed 9 years ago by
- Status changed from new to needs_review
Changed 9 years ago by
comment:2 Changed 9 years ago by
The previous patch had a silly typo and some trailing whitespace, so here's a new one.
comment:3 Changed 9 years ago by
- Cc AlexGhitza added
comment:4 Changed 9 years ago by
- Keywords sd40.5 added
- Reviewers set to Karl-Dieter Crisman
Comments:
- I can't give the new code itself positive review, although it seems correct (mostly I have questions about the change to
absolute_degree
. Maybe that could be moved to a different ticket if one doesn't find a reviewer? - I can, however, suggest a trivial fix - the second-to-last line below had
new_codomain
, notnew_domain
(I'd add a patch, but see the previous comment):def extend_domain(self, new_domain): r""" INPUT: - ``self`` -- a member of Hom(Y, Z) - ``new_domain`` -- an object X such that there is a canonical coercion `\phi` in Hom(X, Y)
Also, maybe we want to just turn that double-underscore attribute into a single-underscore attribute? Here at SD 40.5 that is being suggested strongly for any occurrences of such things. - In the nearly identical two occurrences of
self.__maximal_order[v] = RelativeOrder(self, abs_order, is_maximal=True, check=False)
is it possible that the first one is not supposed to have the[v]
?
comment:5 Changed 9 years ago by
- Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, William Stein
comment:6 Changed 9 years ago by
Hi,
The new code looks fine, though the new v option should be documented in the docstring:
INPUT: - ``v`` - (default: None) None, a prime, or a list of primes. - if v is None, return the maximal order. - if v is a prime, return an order that is p-maximal. - if v is a list, return an order that is maximal at each prime in the list v.
So if somebody adds the above to the docstring, then we should give this a positive review.
comment:7 Changed 9 years ago by
I documented the input for maximal_order
. Apply trac_12767_INPUT.patch and it should be good to go.
comment:8 Changed 9 years ago by
- Reviewers changed from Karl-Dieter Crisman, William Stein to Karl-Dieter Crisman, William Stein, Benjamin Jones
comment:9 Changed 9 years ago by
- Status changed from needs_review to positive_review
comment:10 Changed 9 years ago by
- Merged in set to sage-5.1.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
Patch against 5.0.beta10