Numerical approximation of an algebraic number raises a ValueError

[slabbe]

Numerical approximation works for complex numbers:

sage: n(1 + I)
1.00000000000000 + 1.00000000000000*I
sage: (1 + I).n()
1.00000000000000 + 1.00000000000000*I

but not for algebraic numbers:

sage: m = matrix(3, [3,1,6,5,2,9,7,3,13])
sage: E = m.eigenvalues()
sage: E
[18.16815365088822?, -0.08407682544410650? - 0.2190261484802906?*I, -0.08407682544410650? + 0.2190261484802906?*I]
sage: map(type, E)
[<class 'sage.rings.qqbar.AlgebraicNumber'>, <class 'sage.rings.qqbar.AlgebraicNumber'>, <class 'sage.rings.qqbar.AlgebraicNumber'>]
sage: map(n, E)
Traceback (most recent call last):
...
ValueError: Cannot coerce algebraic number with non-zero imaginary part to algebraic real

[spice]

The patch provides a short and sweet fix to the above problem. The issue arises in sage/misc/functional.py's numerical_approx() function:

if not (is_ComplexNumber(x) or is_ComplexDoubleElement(x)):
            try:
                return sage.rings.real_mpfr.RealField(prec)(x)                                                                              
            except TypeError:
                pass
        return sage.rings.complex_field.ComplexField(prec)(x)

Attempting to call RealField?() on a complex AlgebraicNumber? raises a ValueError? and not a TypeError?, so the exception is not caught. Changing the except clause to catch all errors fixes this:

if not (is_ComplexNumber(x) or is_ComplexDoubleElement(x)):
            try:
                return sage.rings.real_mpfr.RealField(prec)(x)
            # Trac 10761: now catches all errors (instead of just                                                 
            # a TypeError), since calling RealField on AlgebraicNumbers                                           
            # can raise a ValueError                                                                              
            except:
                pass
        return sage.rings.complex_field.ComplexField(prec)(x)

[rbeezer]

Hi Simon,

This will be very useful, thanks for digging it up. Three comments:

1. type(E[1]) would be more accurate (and readable) as E[1].parent(), more Sage-like.

2. I write lots of "naked" except clauses, which I think is a bad practice. Is this a place where could just add ", Value Error" after the TypeError?

3. Style Points: I think three lines of source comments for this fix is more than you would normally see. With the Trac number in the docstring, and a patch on Trac, you don't need to say so much. With one line here, it'd warn anybody away from messing with it.

Passes all long tests right now.

Rob

[spice]

Hi Rob

On the issues you've raised:

1) type(E[1]) changed to E[1].parent() in the example as per your suggestion.

2) Regarding the naked except clause, I tried to add a catch of ValueError, but the function was still breaking. I wasn't able to figure out why, which is why I defaulted to catching everything. I figured this was okay, though, since this was just one part of a multistep coercion attempt - and thus errors are to be expected. Thoughts?

3) Inline comment streamlined to a single line.

Simon

[rbeezer]

Simon,

Looks real good. If the precise error list does not do the trick, then I think leaving it empty is fine.

Applies and builds, passes tests, docs build and look good. Positive review.

Rob

[kini]

This works for me...

sage/misc/functional.py

@@ -1260 +1270 @@
         if not (is_ComplexNumber(x) or is_ComplexDoubleElement(x)):
             try:
                 return sage.rings.real_mpfr.RealField(prec)(x)
-            except TypeError:
+            except (TypeError, ValueError):
                 pass
         return sage.rings.complex_field.ComplexField(prec)(x)

I think it would be cleaner than just catching all exceptions. Simon, what exactly was wrong with catching ValueErrors??

[slabbe]

Simon, what exactly was wrong with catching ValueErrors???

I was going to ask the same, but kini beated me.

I also was going to suggest this if the tuple didn't work :

except TypeError:
    pass
except ValueError:
    pass

but I think what kini proposes is best. No?

[kini]

Did you do something like this?

except TypeError, ValueError:

This will attempt to catch TypeError exceptions and assign them to the variable name ValueError. It's ​a deprecated behavior from old versions of Python.

[spice]

Replaces existing patch.

[spice]

Ah. n00b error - I forgot to add parentheses. It works now.

Simon

[kini]

Er, sorry, I of course meant to link to ​this page. Patch looks good.

[slabbe]

Thank you Simon for the fix !

Sébastien