Opened 12 years ago
Closed 12 years ago
#1805 closed defect (fixed)
[with patch; Positive review] improve doctest coverage for Factorization; fix several critical bugs
Reported by: | was | Owned by: | ncalexan |
---|---|---|---|
Priority: | major | Milestone: | sage-2.10.3 |
Component: | doctest coverage | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Attachments (6)
Change History (20)
comment:1 Changed 12 years ago by
- Owner changed from failure to was
- Status changed from new to assigned
Changed 12 years ago by
comment:2 follow-up: ↓ 9 Changed 12 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 12 years ago by
this part 2 fixes some additional issues, typos, etc., I saw when proofreading my patch.
comment:3 Changed 12 years ago by
- Summary changed from improve doctest coverage for structure/factorization.py to [with patch; needs review] improve doctest coverage for structure/factorization.py
comment:4 Changed 12 years ago by
- Owner changed from was to ncalexan
- Status changed from assigned to new
- Summary changed from [with patch; needs review] improve doctest coverage for structure/factorization.py to [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:5 Changed 12 years ago by
Nick, can you give this another review?
Cheers,
Michael
comment:6 Changed 12 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 12 years ago by
These patches don't import cleanly against current. They also need the bug doctest removed.
comment:8 Changed 12 years ago by
- Summary changed from [with patch; needs to be merged with 1804] improve doctest coverage for structure/factorization.py to [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 12 years ago by
Changed 12 years ago by
comment:9 in reply to: ↑ 2 Changed 12 years ago by
- Summary changed from [with patch; needs review] improve doctest coverage for Factorization; fix several critical bugs to [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 12 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 12 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 12 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 12 years ago by
this should be the final of four patches; it fixes one problem found by the referee (gfurnish)
comment:13 Changed 12 years ago by
- Summary changed from [with patch; Positive review pending change] improve doctest coverage for Factorization; fix several critical bugs to [with patch; Positive review] improve doctest coverage for Factorization; fix several critical bugs
I attached the one small change requested.
comment:14 Changed 12 years ago by
- Resolution set to fixed
- Status changed from new to closed
Merged in Sage 2.10.3.rc1
Before:
After:
and I fixed numerous conceptual bugs/mistakes in that file.