repr of NumberFields (the parents) should indicate its embedding if there is one

[mkoeppe]

As discussed in #21105, number fields with coercion embeddings, in particular with real embeddings, behave quite differently from those without - but there's no indication of embeddings in the print representation:

sage: K.<a> = NumberField(x^2 - 2)
sage: a.parent()
Number Field in a with defining polynomial x^2 - 2
sage: K.<sqrt2> = NumberField(x^2 - 2, embedding=1.4)
sage: sqrt2.parent()
Number Field in sqrt2 with defining polynomial x^2 - 2

This ticket changes the print representation when there is an embedding as follows.

Number Field in a with defining polynomial x^13 - 2/3*x + 3 with a = -1.106745229567614?

This also works well for more complicated situations such as this one:

            sage: K.<a> = NumberField(x^4 - 3); K
            Number Field in a with defining polynomial x^4 - 3
            sage: H.<b>, from_H = K.subfield(a^2)
            sage: H
            Number Field in b with defining polynomial x^2 - 3 with b = a^2

CC: @videlec @jplab @JohnCremona @tscrim @mezzarobba @jdemeyer

Component: number fields

Author: Matthias Koeppe

Branch: d404006

Reviewer: Jean-Philippe Labbé

Issue created by migration from https://trac.sagemath.org/ticket/21161

[videlec]

comment:2

Shorter sentence would be better.

Real Number Field in sqrt2 as the root of x^2 - 2 in [1.41, 1.42]

Ideally (as above), the X in near X should have the form of an interval [1.41,1.42] that specifies uniquely the root.

[videlec]

comment:3

Or possibly

Real Number Field in sqrt2=1.41421356237309? with defining polynomial x^2 - 2

[mkoeppe]

comment:4

AlgebraicGenerator (from qqbar) uses

   Number Field in a with defining polynomial y^2 - y - 1 with a in 1.618033988749895?

[videlec]

comment:5

Replying to @mkoeppe:

AlgebraicGenerator (from qqbar) uses

   Number Field in a with defining polynomial y^2 - y - 1 with a in 1.618033988749895?

Which is not that bad except the a in 1.618033988749895?. Would make more sense with a in [1.61, 1.62] or a = 1.618033988749895?.

[nbruin]

comment:7

This should work for number fields with a real embedding as well as a complex one. The interval notation is painful for the latter, so "with a=..." would probably work better. Also good if we end up with number fields with a p-adic embedding specified:

K.<a>=NumberField(x^2+1,embedding=pAdicField(5)(-1).sqrt())

[mkoeppe]

Branch: u/mkoeppe/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

d40c9e6

NumberField_generic._repr_: Print embedding if there is one

[sagetrac-git]

Commit: d40c9e6

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ba3b35d

NumberField_generic._repr_: Print embedding if there is one

[sagetrac-git]

Changed commit from d40c9e6 to ba3b35d

[mkoeppe]

comment:11

In this patch I am using a format with with a = ... now (as suggested by nbruin), which works well also for the complex case:

Number Field in a with defining polynomial x^13 - 2/3*x + 3 with a = -1.106745229567614?

and for more complicated situations such as this one:

            sage: K.<a> = NumberField(x^4 - 3); K
            Number Field in a with defining polynomial x^4 - 3
            sage: H.<b>, from_H = K.subfield(a^2)
            sage: H
            Number Field in b with defining polynomial x^2 - 3 with b = a^2

The output for the following looks a bit strange because printing goes through sage.rings.real_lazy.LazyBinop:

sage: QuadraticField(-1, 'a')
Number Field in a with defining polynomial x^2 + 1 with a = 1*I

Should we special case this?

[sagetrac-git]

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

ed3aab0

Update doctest outputs

[sagetrac-git]

Changed commit from ba3b35d to ed3aab0

[mkoeppe]

Author: Matthias Koeppe

[jplab]

Changed branch from u/mkoeppe/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one to public/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one

[jplab]

comment:14

The patchbot gave 2 pyflakes warnings, I repaired one of them, the other one I can not make sure that it does not have side effects.

Otherwise, the patchbot is happy and the resolution looks reasonable. I set it as positive review, if you agree @mkoeppe.

---

New commits:

875731c

fixed one pyflakes

[jplab]

Reviewer: Jean-Philippe Labbé

[jplab]

Changed commit from ed3aab0 to 875731c

[jplab]

comment:15

Whoooops!

[sagetrac-git]

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

76c384e

Fix up doctest after change

[sagetrac-git]

Changed commit from 9ce74e3 to 76c384e

[mkoeppe]

comment:26

Fixed these two.

[sagetrac-git]

Changed commit from 76c384e to c4c0088

[sagetrac-git]

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

c4c0088

Merge tag '8.8.beta5' into t/21161/public/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one

[jplab]

comment:32

There are only 6 failing doctests. That's not bad!

----------------------------------------------------------------------
sage -t --long src/sage/rings/qqbar.py  # 5 doctests failed
sage -t --long src/sage/rings/number_field/order.py  # 1 doctest failed
----------------------------------------------------------------------

[sagetrac-git]

Changed commit from c4c0088 to d404006

[sagetrac-git]

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

c1e1514	Merge tag '8.8.beta6' into t/21161/public/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one
d404006	Fix doctests

[jplab]

comment:35

Looks good to me. The tests seem to pass on the latest beta and the pyflakes errors are not regressions. I set this to positive review.

[vbraun]

Changed branch from public/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one to d404006

[fchapoton]

Changed commit from d404006 to none

[fchapoton]

comment:37

This ticket is probably causing (randomly) infinite loops on several patchbots.

For example, see

https://patchbot.sagemath.org/log/27408/Ubuntu/18.04/x86_64/4.15.0-52-generic/petitbonum/2019-06-20%2014:50:38?short

EDIT:
the problematic doctest is

File "src/sage/structure/parent.pyx", line 1734, in sage.structure.parent.Parent.hom.register_embedding
Failed example:
    K.coerce_embedding()(a)
Exception raised:
...
RuntimeError: maximum recursion depth exceeded while calling a Python object

[jplab]

comment:38

Replying to @fchapoton:

This ticket is probably causing (randomly) infinite loops on several patchbots.

For example, see

https://patchbot.sagemath.org/log/27408/Ubuntu/18.04/x86_64/4.15.0-52-generic/petitbonum/2019-06-20%2014:50:38?short

EDIT:
the problematic doctest is

File "src/sage/structure/parent.pyx", line 1734, in sage.structure.parent.Parent.hom.register_embedding
Failed example:
    K.coerce_embedding()(a)
Exception raised:
...
RuntimeError: maximum recursion depth exceeded while calling a Python object

The source code that changed is

--- a/src/sage/rings/number_field/number_field.py
+++ b/src/sage/rings/number_field/number_field.py
@@ -3087,9 +3089,16 @@ class NumberField_generic(WithEqualityById, number_field_base.NumberField):
         """
-        return "Number Field in %s with defining polynomial %s"%(
-                   self.variable_name(), self.polynomial())
+        result = "Number Field in {} with defining polynomial {}".format(self.variable_name(), self.polynomial())
+        gen = self.gen_embedding()
+        if gen is not None:
+            result += " with {} = {}".format(self.variable_name(), gen)
+        return result

The rest of the diff consist of adapting the doctests. Where could the infinite loop come from? Hmm.

[fchapoton]

comment:39

It appears that using the embedding in the repr is a very bad idea...

Even when this problematic doctest works, calling

K.coerce_embedding()

after this doctest makes sage crash..
And the culprit are really those 2 "innocent" added lines..

[fchapoton]

comment:40

I have made #28036 to revert the changes made here.

[fchapoton]

comment:41

Maybe changing this line would be a way out:

return copy(self._embedding) # It might be overkill to make a copy here