Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken conversion from ZZ['x'] to Qp['y'] #29828

Closed
EnderWannabe opened this issue Jun 8, 2020 · 16 comments
Closed

Broken conversion from ZZ['x'] to Qp['y'] #29828

EnderWannabe opened this issue Jun 8, 2020 · 16 comments

Comments

@EnderWannabe
Copy link
Contributor

Currently, the following code fails:

sage: R.<x> = PolynomialRing(ZZ)
sage: f = x^4 - x - 1
sage: S.<y> = PolynomialRing(Qp(5))
sage: g2 = S(f)
sage: g2.factor()

TypeError: 'sage.rings.fraction_field_element.FractionFieldElement_1poly_field' object is not iterable

However, the following code works

sage: g = y^4 - y - 1
sage: g.factor()

and moreover

sage: g == g2
True

The cause seems to be the following:

sage: g2._poly
x^4 - x - 1

To fix this, we change the line where ._poly is initialized when converting from ZZ[] to Qp[].

CC: @bhutz @pfili

Component: padics

Keywords: gsoc20

Author: Alexander Galarraga

Branch/Commit: 726c83b

Reviewer: Travis Scrimshaw

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

@EnderWannabe EnderWannabe added this to the sage-9.2 milestone Jun 8, 2020
@EnderWannabe
Copy link
Contributor Author

@EnderWannabe

This comment has been minimized.

@EnderWannabe
Copy link
Contributor Author

Commit: d2fd21f

@EnderWannabe
Copy link
Contributor Author

New commits:

d2fd21fFix for conversion from ZZ[] to Qp[]

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

Changed commit from d2fd21f to 12a0454

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

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

12a0454Better fix

@EnderWannabe

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Jun 9, 2020

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jun 9, 2020

comment:5

The fix is good. Just add a doctest showing the issue is fixed and prevent a regression.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

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

d7975d5Added doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

Changed commit from 12a0454 to d7975d5

@EnderWannabe
Copy link
Contributor Author

comment:7

Replying to @tscrim:

The fix is good. Just add a doctest showing the issue is fixed and prevent a regression.

Doctest added. Tests multiplication since this also breaks without the fix

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

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

726c83bBetter test

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2020

Changed commit from d7975d5 to 726c83b

@tscrim
Copy link
Collaborator

tscrim commented Jun 10, 2020

comment:9

Thank you. LGTM.

@vbraun
Copy link
Member

vbraun commented Jul 8, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants