Opened 3 years ago

Closed 3 years ago

#29266 closed defect (fixed)

Bugfix concerning ticket #26421

Reported by: soehms Owned by:
Priority: critical Milestone: sage-9.1
Component: commutative algebra Keywords: factorization, integral domain
Cc: tscrim Merged in:
Authors: Sebastian Oehms Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: a21db27 (Commits, GitHub, GitLab) Commit: a21db2793711b9bd10127911bc9a2c432dca9947
Dependencies: Stopgaps:

GitHub link to the corresponding issue


In #26421 factorization of polynomials has been extended to previously untreated cases in which the polynomial's base ring is an integral domain. The factorization is done over the field of fraction and excepted if it can be coerced into the base ring itself. Since the method base_change of the class Factorzation doesn't check if the unit of the factorization is coerced to a unit again, this implementation is too vastly:

sage: R.<t> = LaurentPolynomialRing(ZZ)
sage: P.<x> = R[]
sage: f = 2*x + 4
sage: f.is_irreducible()
sage: F = f.factor(); F
(2) * (x + 2)
sage: F.unit()

Change History (6)

comment:1 Changed 3 years ago by soehms

Branch: u/soehms/factorization_integral_domain_29266

comment:2 Changed 3 years ago by soehms

Authors: Sebastian Oehms
Commit: 004c1c4211af86c177b8fd537f6aad01cc593623
Status: newneeds_review

Maybe a better place for the fix would be the method base_change of the class Factorization. But since this leads to the failure of previous doctests (in cases is_unit is not implemented), I preferred the implementation in method factor.

New commits:

004c1c429266 initial version

comment:3 Changed 3 years ago by tscrim

Reviewers: Travis Scrimshaw

One little change: "coerce" to "convert". If it was a coercion map, then the fact it was a unit must be preserved (as coercions must be morphisms). Once you change that, you can set a positive review.

comment:4 Changed 3 years ago by git

Commit: 004c1c4211af86c177b8fd537f6aad01cc593623a21db2793711b9bd10127911bc9a2c432dca9947

Branch pushed to git repo; I updated commit sha1. New commits:

a21db2729266: correction according to review

comment:5 Changed 3 years ago by soehms

Status: needs_reviewpositive_review


comment:6 Changed 3 years ago by vbraun

Branch: u/soehms/factorization_integral_domain_29266a21db2793711b9bd10127911bc9a2c432dca9947
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.