Opened 12 years ago

Closed 12 years ago

#10907 closed defect (fixed)

Factorization is not coercion-friendly

Reported by: Luis Felipe Tabera Alonso Owned by: Robert Bradshaw
Priority: major Milestone: sage-4.7
Component: factorization Keywords: Factorization
Cc: Merged in: sage-4.7.alpha2
Authors: Francis Clarke Reviewers: Luis Felipe Tabera Alonso
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

I get the following:

sage: K = QQ[x]
sage: f1 = K.random_element()
sage: f2 = K.random_element()
sage: f= f1*f2               
sage: F = factor(f)
sage: f-F
...
TypeError: unsupported operand parent(s) for '-': 'Univariate Polynomial Ring in x over Rational Field' and '<class 'sage.structure.factorization.Factorization'>'
sage: F-f
0
sage: f == F
False
sage: F in K
False

I think that at least f-F should work.

Attachments (1)

trac_10907.patch (1.1 KB) - added by Francis Clarke 12 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 12 years ago by Francis Clarke

Note that (following on from the above)

sage: F - f
0

Including definitions for __radd__ and __rsub__ for the sage.structure.factorization.Factorization class should fix the f - F problem.

The other two results seem reasonable to me.

Changed 12 years ago by Francis Clarke

Attachment: trac_10907.patch added

comment:2 Changed 12 years ago by Francis Clarke

Authors: Francis Clarke
Component: coercionfactorization
Keywords: coercion removed
Status: newneeds_review

The attached patch allows right addition and right subtraction for factorizations.

comment:3 Changed 12 years ago by Luis Felipe Tabera Alonso

Reviewers: Luis Felipe Tabera Alonso
Status: needs_reviewpositive_review

Wow, thanks!

The code is simple, a direct translation of add and sub

The code passes doctest. The issue is fixed and I have tested the code in other situations (like different parent for the operands) and everithing works as expected.

Concerning the other issues I can live with them.

Positive review

comment:4 Changed 12 years ago by Jeroen Demeyer

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