Opened 10 years ago
Closed 10 years ago
#6040 closed enhancement (fixed)
[with patch, positive review] Added Doctests for QuadraticForms methods
Reported by: | jonhanke | Owned by: | justin |
---|---|---|---|
Priority: | major | Milestone: | sage-4.0 |
Component: | quadratic forms | Keywords: | quadraticform |
Cc: | mabshoff, wstein, tornaria | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Adding Doctests to bring coverage up to 100%.
Attachments (3)
Change History (14)
Changed 10 years ago by
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
- Summary changed from Added Doctests for QuadraticForms methods to [with patch, needs review] Added Doctests for QuadraticForms methods
comment:3 Changed 10 years ago by
Additional patch to bring QuadraticForm? doctests to 100%.
Known Issues:
Several doctests fail because of the need for hilbert_symbol() to accept rational numbers, and similarly for IsPadicSquare?(). These should be addressed by the changes made in Ticket #5834.
Changed 10 years ago by
comment:4 Changed 10 years ago by
comment:5 Changed 10 years ago by
- Description modified (diff)
Also the patch in Ticket #6037 (rewrite and careful documentation of local density routines) is related to getting the doctest coverage to 100%.
comment:6 Changed 10 years ago by
So, it turns out there are 4 patches in this series, and they must be applied in order. In particular, patch-3 depends on patch-2, which is at #6037, but I misunderstood that. The correct sequence is to apply patch-1 in #5954, then patch-2 in #6037, then patch-3 and patch-4 in this ticket.
If that order is followed, the patch sequence applies cleanly to 3.4.1 as well as 4.0.alpha0.
comment:7 Changed 10 years ago by
Some doctests were broken on 4.0.alpha0 + patch-1 (#5954) + patch-2 (#6037) + patch-3 + patch-4.
All doctests pass for me when adding on top of that
- attachment:patch-5__QF_reviewer__4.0.alpha0.patch
- patch in #6059
- patch in #6062
- patch in #6064
Note that the patch-3__QF_misc_doctests__4.0.alpha0.patch
I uploaded earlier is a mistake, just ignore it.
comment:8 Changed 10 years ago by
Note: the patch-5 also adds a few "#long time" comments to file quadratic_form__local_representation_conditions.py
. Skipping these reduces the time for doctesting the whole file from 48 s to 20 s.
comment:9 Changed 10 years ago by
- Summary changed from [with patch, needs review] Added Doctests for QuadraticForms methods to [with patch, positive review] Added Doctests for QuadraticForms methods
Add positive review due to Gonzalo.
Cheers,
Michael
comment:10 Changed 10 years ago by
I'm ok with the positive review. I'll list the renamed/removed functions as for the other tickets, and I'll post a ticket to add compatibility functions with deprecation warnings.
hanke_mass__maximal
was removed.GHY_mass_maximal
was renamedGHY_mass__maximal
.conway_generic_mass
was removed.conway_p_mass_adjustment
was removed.local_diagonal
was removed.find_primitive_p_divisible_vector__all
was removed.
comment:11 Changed 10 years ago by
- Milestone changed from sage-4.0.1 to sage-4.0
- Resolution set to fixed
- Status changed from new to closed
Merged in Sage 4.0.rc0.
Cheers,
Michael
Note: There are currently two broken doctests in this patch (using the older routine IsPadic? Square()), which should resolve themselves once Cremona's patch (Ticket #5834) is applied.