Opened 12 years ago

Closed 12 years ago

#9336 closed defect (fixed)

Improve doctest coverage for sage/rings/number_field

Reported by: David Loeffler Owned by: David Loeffler
Priority: major Milestone: sage-4.5.2
Component: number fields Keywords: doctest
Cc: John Cremona Merged in: sage-4.5.2.alpha0
Authors: David Loeffler Reviewers: John Cremona, Francis Clarke
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by David Loeffler)

The patch below adds about 50 new doctests in sage/rings/number_field, particularly in the files maps.py, morphism.py and number_field_morphisms.pyx.

Attachments (1)

trac_9336.patch (55.7 KB) - added by David Loeffler 12 years ago.
patch against 4.4.4

Download all attachments as: .zip

Change History (9)

comment:1 Changed 12 years ago by David Loeffler

Description: modified (diff)

comment:2 Changed 12 years ago by David Loeffler

Status: newneeds_review

comment:3 Changed 12 years ago by John Cremona

Authors: David Loeffler
Reviewers: John Cremona
Status: needs_reviewpositive_review

Good job! Patch applies to 4.4.4 and tests pass and docs build fine.

After this the number_fields directory scores as follows:

SCORE sage/rings/number_field//unit_group.py: 100% (15 of 15)
SCORE sage/rings/number_field//number_field_element.pyx: 81% (91 of 111)
SCORE sage/rings/number_field//totallyreal.pyx: 100% (4 of 4)
SCORE sage/rings/number_field//number_field_base.pyx: 72% (8 of 11)
SCORE sage/rings/number_field//order.py: 100% (62 of 62)
SCORE sage/rings/number_field//number_field_element_quadratic.pyx: 95% (42 of 44)
SCORE sage/rings/number_field//number_field.py: 100% (183 of 183)
SCORE sage/rings/number_field//number_field_ideal.py: 92% (72 of 78)
SCORE sage/rings/number_field//totallyreal_dsage.py: 13% (2 of 15)
SCORE sage/rings/number_field//maps.py: 100% (23 of 23)
SCORE sage/rings/number_field//number_field_rel.py: 91% (62 of 68)
SCORE sage/rings/number_field//totallyreal_data.pyx: 100% (7 of 7)
SCORE sage/rings/number_field//small_primes_of_degree_one.py: 100% (4 of 4)
SCORE sage/rings/number_field//galois_group.py: 100% (29 of 29)
SCORE sage/rings/number_field//number_field_ideal_rel.py: 78% (22 of 28)
SCORE sage/rings/number_field//totallyreal_rel.py: 60% (3 of 5)
SCORE sage/rings/number_field//morphism.py: 100% (22 of 22)
SCORE sage/rings/number_field//class_group.py: 72% (13 of 18)
SCORE sage/rings/number_field//number_field_morphisms.pyx: 100% (13 of 13)
SCORE sage/rings/number_field//totallyreal_phc.py: 100% (2 of 2)

comment:4 Changed 12 years ago by Francis Clarke

Status: positive_reviewneeds_work

Just one little niggle: according to the developer's guide, code like

        try: 
            return self.list()[0] 
        except: 
            raise ValueError, "Set is empty"

(lines 91 to 94 in the patched morphism.py) should be avoided. Better would be

        try: 
            return self[0] 
        except IndexError: 
            raise ValueError, "Set is empty"

comment:5 Changed 12 years ago by John Cremona

OK, I agree that would be better.

Changed 12 years ago by David Loeffler

Attachment: trac_9336.patch added

patch against 4.4.4

comment:6 Changed 12 years ago by David Loeffler

Status: needs_workneeds_review

Here's a new patch, which explicitly checks whether the length of the list is 0 rather than using try/except.

comment:7 in reply to:  6 Changed 12 years ago by Francis Clarke

Reviewers: John CremonaJohn Cremona, Francis Clarke
Status: needs_reviewpositive_review

Replying to davidloeffler:

Here's a new patch ...

That works fine, so back to positive review.

comment:8 Changed 12 years ago by Mitesh Patel

Merged in: sage-4.5.2.alpha0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.