Opened 15 years ago
Closed 15 years ago
#1805 closed defect (fixed)
[with patch; Positive review] improve doctest coverage for Factorization; fix several critical bugs
Reported by: | William Stein | Owned by: | ncalexan |
---|---|---|---|
Priority: | major | Milestone: | sage-2.10.3 |
Component: | doctest coverage | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Attachments (6)
Change History (20)
comment:1 Changed 15 years ago by
Owner: | changed from failure to William Stein |
---|---|
Status: | new → assigned |
Changed 15 years ago by
Attachment: | trac-1805.patch added |
---|
comment:2 follow-up: 9 Changed 15 years ago by
This is a preliminary patch. I'm now doctesting all of sage-2.10 with this patch applied to see what else is broke. I'll post another patch that fixes the problems later.
Changed 15 years ago by
Attachment: | trac-1805-part2.patch added |
---|
this part 2 fixes some additional issues, typos, etc., I saw when proofreading my patch.
comment:3 Changed 15 years ago by
Summary: | improve doctest coverage for structure/factorization.py → [with patch; needs review] improve doctest coverage for structure/factorization.py |
---|
comment:4 Changed 15 years ago by
Owner: | changed from William Stein to ncalexan |
---|---|
Status: | assigned → new |
Summary: | [with patch; needs review] improve doctest coverage for structure/factorization.py → [with patch; needs to be merged with 1804] improve doctest coverage for structure/factorization.py |
I fixed 1804 not knowing that 1805 did a lot of the same work. Damn! They should both be applied but of course they need to be merged. I'll try to do it tomorrow.
comment:6 Changed 15 years ago by
Yes, please review it. My patch fixes a number of subtle serious bugs involving pickling factorizations, which could cause people problems.
comment:7 Changed 15 years ago by
These patches don't import cleanly against current. They also need the bug doctest removed.
comment:8 Changed 15 years ago by
Summary: | [with patch; needs to be merged with 1804] improve doctest coverage for structure/factorization.py → [with patch; needs review] improve doctest coverage for Factorization; fix several critical bugs |
---|
I've attached a brand new rebased patch which also fixes a critical bug in factorization (!) which may expose a bug in totallyrealfield, by the way.
I also changed factorizations to be immutable, as suggested by the referee, and they now no longer derive from list, so that cmp works correctly.
Changed 15 years ago by
Attachment: | sage-1805-part2-rebased.patch added |
---|
Changed 15 years ago by
Attachment: | sage-1805-rebase_part3.patch added |
---|
comment:9 Changed 15 years ago by
Summary: | [with patch; needs review] improve doctest coverage for Factorization; fix several critical bugs → [with patch; Positive review pending change] improve doctest coverage for Factorization; fix several critical bugs |
---|
sage -t devel/sage-patch1805/build/sage/structure/factorization.py File "factorization.py", line 602:
sage: F = factor(-2*x^{2 - 1); F }
Expected:
(-2.0) * (1.0*x^{2 + 0.5) * (1.0*x}2 + 1.11022302463e-16*x + 0.5)
Got:
(-2.0) * (1.0*x^{2 - 2.22044604925e-16*x + 0.5) * (1.0*x}2 + 0.5)
Positive review pending fix.
comment:10 Changed 15 years ago by
{{{sage -t devel/sage-patch1805/build/sage/structure/factorization.py File "factorization.py", line 602:
sage: F = factor(-2*x^{2 - 1); F }
Expected:
(-2.0) * (1.0*x^{2 + 0.5) * (1.0*x}2 + 1.11022302463e-16*x + 0.5)
Got:
(-2.0) * (1.0*x^{2 - 2.22044604925e-16*x + 0.5) * (1.0*x}2 + 0.5)
}}}
comment:11 Changed 15 years ago by
[[[ sage -t devel/sage-patch1805/build/sage/structure/factorization.py File "factorization.py", line 602:
sage: F = factor(-2*x^{2 - 1); F }
Expected:
(-2.0) * (1.0*x^{2 + 0.5) * (1.0*x}2 + 1.11022302463e-16*x + 0.5)
Got:
(-2.0) * (1.0*x^{2 - 2.22044604925e-16*x + 0.5) * (1.0*x}2 + 0.5)
]]]
comment:12 Changed 15 years ago by
sage -t devel/sage-patch1805/build/sage/structure/factorization.py********************************************************************** File "factorization.py", line 602: sage: F = factor(-2*x^2 - 1); F Expected: (-2.0) * (1.0*x^2 + 0.5) * (1.0*x^2 + 1.11022302463e-16*x + 0.5) Got: (-2.0) * (1.0*x^2 - 2.22044604925e-16*x + 0.5) * (1.0*x^2 + 0.5) **********************************************************************
Changed 15 years ago by
Attachment: | sage-1805-part4-make_ref_happy.patch added |
---|
this should be the final of four patches; it fixes one problem found by the referee (gfurnish)
comment:13 Changed 15 years ago by
Summary: | [with patch; Positive review pending change] improve doctest coverage for Factorization; fix several critical bugs → [with patch; Positive review] improve doctest coverage for Factorization; fix several critical bugs |
---|
I attached the one small change requested.
comment:14 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Merged in Sage 2.10.3.rc1
Before:
After:
and I fixed numerous conceptual bugs/mistakes in that file.