Get rid of the bogus coercion from SR to QQbar

[tmonteil]

Original issue: there is no problem buildind a polynomial over AA:

sage: R = AA[x]                       
sage: P = R(x^6+x^5+x^4-x^3+x^2-x+2/5)

But the same operation does not work on QQbar:

sage: S = QQbar[x]
sage: Q = S(x^6+x^5+x^4-x^3+x^2-x+2/5)
Traceback (most recent call last)
...
NotImplementedError: symbol

Hovewer, this workaround works:

sage: Q = P.change_ring(QQbar)

Moreover the following works :

sage: S.<x> = PolynomialRing(QQbar,'x')
sage: Q = S(x^6+x^5+x^4-x^3+x^2-x+2/5)
sage: S == QQbar[x]
True

Weird isn't it ?

[nbruin]

The weirdness is really just limited to the difference in behaviour between these:

sage: AA['x'](var('x'))
x
sage: QQbar['x'](var('x'))
NotImplementedError: symbol

The thing that's failing in the latter case is

QQbar._element_constructor_(x)

and

AA._element_constructor_(x)

fails in exactly the same way. That suggest that in the case of AA['x'] something is tried before resorting to the thing that fails for QQbar['x'].

For instance,

AA['y'](var('x'))
QQbar['y'](var('x'))

both (rightly) fail,but with significantly different tracebacks. This might give some indication of what is tried for AA but not for QQbar.

[nbruin]

The thing that goes wrong is the following:

sage: S=QQbar['x']
sage: S.base_ring().has_coerce_map_from(SR)
True

Once this is found, it's clear how an element of SR should be converted into QQbar['x']: SR already coerces into QQbar itself, so we should just convert the element into a "constant polynomial".

Except, of course, that SR doesn't really coerce into QQbar: Only "constant" expressions might (and even then, of course pi doesn't), and x of course isn't one of those. So the proper solution is probably to NOT install a coercion from SR to QQbar, because it isn't a coercion anyway. It's only a partial map. Doing so is probably going to be painful for something else.

This problem doesn't arise for AA because it's pretty clear that not even algebraic elements fit in AA, so no coercion is installed.

[nbruin]

Don't pretend there is a coercion from the symbolic ring into QQbar

[nbruin]

OK, solving a ticket by deleting two lines is just too appealing to pass by. Of course, there is are all kinds of things that fail this way, e.g.

QQbar(1)+sqrt(2)

doesn't work anymore, but QQbar(1)+QQbar(sqrt(2)) of course still does.

[mmezzarobba]

I was going to report another problem caused by the bogus coercion from SR:

sage: CC.has_coerce_map_from(QQbar)
True
sage: QQbar.has_coerce_map_from(SR)
True
sage: CC.has_coerce_map_from(SR)
False

So I'm all in favor of removing it, even if it breaks stuff that should never have worked in the first place.

We might want to put (or actually put back?) a coercion in the other direction, since symbolic expressions can embed elements of QQbar. But SR has coerce maps from RR, CC, RLF, etc., which yield other coercions from QQbar by composition. Can we consider that all these maps are the same because they only differ in the "precision" of the result? Or should all coercions from inexact rings to QQbar be removed? (Note that the coercion from QQ to SR has exactly the same problem.)

[mmezzarobba]

As it turns out, it is possible to remove the coercion from SR to QQbar without breaking too many things. Moreover, most of the workarounds I had to add are there to deal with the case of I and will no longer be necessary once #5355 (or #12715) and/or #18036 are fixed (but I suggest we dont't wait fix this to happen).

---

New commits:

​8f85af1	Remove the bogus coercion from SR to QQbar
​8b0a847	#14485 regression test

[git]

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

​9f5b44b

#14485 a few additional tests

[rws]

Patchbot: sage -t --long src/sage/graphs/matchpoly.pyx # 1 doctest failed

[mmezzarobba]

Thanks for the notice. It works here, as well as with the other patchbots. I have no idea at all what is going on.

Jeroen, that sage4 is one of your patchbots, isn't it? Would you mind having a quick look at the failing test?

[vdelecroix]

You should get rid of the function _late_import since it is not used anymore. And it would be better to simplify the code of _coerce_map_from_ as

return from_par == ZZ or from_par == QQ or from_par == int or \
       from_par == long or from_par == AA or from_par == QQbar

Moreover, I am not sure if coercion from int/long is necessary here since a conversion would exists through ZZ.

[mmezzarobba]

Replying to vdelecroix:

You should get rid of the function _late_import since it is not used anymore.

Indeed, I didn't notice, thanks!

And it would be better to simplify the code of _coerce_map_from_ as

return from_par == ZZ or from_par == QQ or from_par == int or \
       from_par == long or from_par == AA or from_par == QQbar

The original style made sense to me so I refrained from making unnecessary changes, but if you prefer this version, fine. I also replaced the == with is.

Moreover, I am not sure if coercion from int/long is necessary here since a conversion would exists through ZZ.

AlgebraicNumber_base.__init__ also has a code path for int and long initializers, but they end up being converted to ZZ, so I guess it won't hurt to remove these cases. Done.

[git]

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

​4ba7202

#14485 simplify code according to reviewer's comments

[vdelecroix]

While working on something else I saw the following comment in symbolic/functions.pyx line 473

# There is no natural coercion from QQbar to the symbolic ring
# in order to support
#     sage: QQbar(sqrt(2)) + sqrt(3)
#     3.146264369941973?
# to work around this limitation, we manually convert
# elements of QQbar to symbolic expressions here     

[mmezzarobba]

Replying to vdelecroix:

While working on something else I saw the following comment in symbolic/functions.pyx line 473

# There is no natural coercion from QQbar to the symbolic ring
# in order to support
#     sage: QQbar(sqrt(2)) + sqrt(3)
#     3.146264369941973?
# to work around this limitation, we manually convert
# elements of QQbar to symbolic expressions here     

The patches on this ticket remove this comment and the corresponding code. I think people now agree that even if it was more or less intentional, trying to support this was a bad idea.

[vdelecroix]

let it be!

[rws]

Replying to mmezzarobba:

Replying to vdelecroix:

While working on something else I saw the following comment in symbolic/functions.pyx line 473

# There is no natural coercion from QQbar to the symbolic ring
# in order to support
#     sage: QQbar(sqrt(2)) + sqrt(3)
#     3.146264369941973?
# to work around this limitation, we manually convert
# elements of QQbar to symbolic expressions here     

The patches on this ticket remove this comment and the corresponding code. I think people now agree that even if it was more or less intentional, trying to support this was a bad idea.

See also #17790, #18832, #20312