Keep embedding info when converting algebraics to number field in projective morphism

[pfili]

Given a map or point defined over QQbar, you can call _numberfield_from_algebraics to return a map over a number field. However, the resulting object cannot be converted back to QQbar since the embedding information is lost.

CC: @bhutz

Component: algebraic geometry

Author: Paul Fili

Branch/Commit: 693578c

Reviewer: David Roe, Ben Hutz

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

[pfili]

Branch: u/paulfili/keep_embedding_info_when_converting_algebraics_to_number_field_in_projective_morphism

[pfili]

comment:2

The following code used to fail:

R.<t>=PolynomialRing(QQ)
s = (t^3+t+1).roots(QQbar)[0][0]
P.<x,y>=ProjectiveSpace(QQbar,1)
H = Hom(P,P)
f = H([s*x^3-13*y^3, y^3-15*y^3])
f_alg = f._number_field_from_algebraics()
f_alg.change_ring(QQbar) # Used to fail

The problem was that the number field being defined over which f_alg was defined should include an embedding, telling Sage where to send the generator in the real/complex numbers. (After all, QQbar elements all have this embedding information.) This information was present but was not being used when defining the number field (possibly due to a quirk in the code from qqbar.py). The fix now updates the code to specify this embedding when creating the number field.

---

New commits:

6c283cf

Redefine the number field used to contain the QQbar elements so that

[pfili]

Author: paulfili

[pfili]

Commit: 6c283cf

[roed314]

Reviewer: David Roe

[roed314]

comment:3

You should add a test to show that the bug has been fixed. You can use

:trac:`23808`

to refer to this ticket in the docstring.

[sagetrac-git]

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

8548426

Added an example illustrating the fix

[sagetrac-git]

Changed commit from 6c283cf to 8548426

[pfili]

comment:5

Replying to @roed314:

You should add a test to show that the bug has been fixed. You can use

:trac:`23808`

to refer to this ticket in the docstring.

Thanks, David, I added the example.

[roed314]

comment:7

You should change print f to just f for Python 3 compatibility. Other than that, looks good and positive review once patchbot approves.

[bhutz]

comment:8

Also, note that there is a similar function for points that should be modified at the same time.

[sagetrac-git]

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

693578c

Changed _number_field_from_algebraics in projective point as well

[sagetrac-git]

Changed commit from 8548426 to 693578c

[pfili]

Changed author from paulfili to Paul Fili

[pfili]

comment:11

Replying to @roed314:

You should change print f to just f for Python 3 compatibility. Other than that, looks good and positive review once patchbot approves.

Thanks, David, I have changed that as well.

[bhutz]

Changed reviewer from David Roe to David Roe, Ben Hutz

[bhutz]

comment:12

This looks fine to me. I also added a description.

[vbraun]

Changed branch from u/paulfili/keep_embedding_info_when_converting_algebraics_to_number_field_in_projective_morphism to 693578c