Opened 13 years ago

Closed 13 years ago

#3840 closed defect (fixed)

[with patch, positive review to the second patch] conversion of 0 from MV polynomial rings broken

Reported by: gfurnish Owned by: somebody
Priority: major Milestone: sage-3.1
Component: basic arithmetic Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The following (and all similar conversions) fail:

RR(RR[x,y](0))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/home/gfurnish/sage-3.1.alpha0-sage.math-only-x86_64-Linux/<ipython console> in <module>()

/home/gfurnish/sage-3.1.alpha0-sage.math-only-x86_64-Linux/real_mpfr.pyx in sage.rings.real_mpfr.RealField.__call__ (sage/rings/real_mpfr.c:3408)()

/home/gfurnish/sage-3.1.alpha0-sage.math-only-x86_64-Linux/multi_polynomial.pyx in sage.rings.polynomial.multi_polynomial.MPolynomial._mpfr_ (sage/rings/polynomial/multi_polynomial.c:1656)()

The attached patch provides doctests and fixes.

Attachments (2)

trac_3840.patch (3.6 KB) - added by gfurnish 13 years ago.
3840-gfurnish-multivariate-conversion-from-0.patch (4.1 KB) - added by ncalexan 13 years ago.

Download all attachments as: .zip

Change History (6)

Changed 13 years ago by gfurnish

comment:1 Changed 13 years ago by was

  • Summary changed from [with patch, needs review] conversion of 0 from MV polynomial rings broken to [with patch, needs work] conversion of 0 from MV polynomial rings broken

REVIEW:

This should be redone by changing the == to <= like this. This is much nicer than using an if statement like in the patch. Redo it.

diff -r 22105a8d4591 sage/rings/polynomial/multi_polynomial.pyx
--- a/sage/rings/polynomial/multi_polynomial.pyx        Wed Aug 13 09:54:40 2008 +0100
+++ b/sage/rings/polynomial/multi_polynomial.pyx        Wed Aug 13 11:02:59 2008 -0700
@@ -15,7 +15,7 @@ cdef class MPolynomial(CommutativeRingEl
     # Some standard conversions
     ####################
     def __int__(self):
-        if self.degree() == 0:
+        if self.degree() <= 0:
             return int(self.constant_coefficient())
         else:
             raise TypeError

comment:2 Changed 13 years ago by ncalexan

Apply only 3840-gfurnish-multivariate-conversion-from-0.patch; hopefully the credit in hg remains with gfurnish. Let me know if not; I used the hg export command this time.

I think this patch does what the referee wants and strikes a better doctesting balance.

Whether -1 is correct or -Infinity is correct, this at least makes sage internally more consistent.

comment:3 Changed 13 years ago by was

  • Summary changed from [with patch, needs work] conversion of 0 from MV polynomial rings broken to [with patch, positive review to the second patch] conversion of 0 from MV polynomial rings broken

Positive review for the second patch. All credit to Gfurnish (except Nick and I get referee credit).

comment:4 Changed 13 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged 3840-gfurnish-multivariate-conversion-from-0.patch in Sage 3.1.alpha2

Note: See TracTickets for help on using tickets.