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)

trac_12767-number_field_docs.patch (10.3 KB) - added by davidloeffler 9 years ago.
Patch against 5.0.beta10
trac_12767_INPUT.patch (980 bytes) - added by benjaminfjones 9 years ago.
document INPUT for maximal_order

Download all attachments as: .zip

Change History (12)

comment:1 Changed 9 years ago by davidloeffler

  • Status changed from new to needs_review

Changed 9 years ago by davidloeffler

Patch against 5.0.beta10

comment:2 Changed 9 years ago by davidloeffler

The previous patch had a silly typo and some trailing whitespace, so here's a new one.

comment:3 Changed 9 years ago by AlexGhitza

  • Cc AlexGhitza added

comment:4 Changed 9 years ago by kcrisman

  • 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, not new_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 was

  • Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, William Stein

comment:6 Changed 9 years ago by was

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.

Changed 9 years ago by benjaminfjones

document INPUT for maximal_order

comment:7 Changed 9 years ago by benjaminfjones

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 benjaminfjones

  • Reviewers changed from Karl-Dieter Crisman, William Stein to Karl-Dieter Crisman, William Stein, Benjamin Jones

comment:9 Changed 9 years ago by benjaminfjones

  • Status changed from needs_review to positive_review

comment:10 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.1.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.