Fix change_ring method of multi-variate polynomials

[kwankyu]

Fixes #36832.

📝 Checklist

• The title is concise, informative, and self-explanatory.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation accordingly.

⌛ Dependencies

[JohnCremona]

Since you are claiming that this fixes #36832, please could you add at least one doctest showing that the errors previously existing have been fixed by this? With that, and assuming that this change does not break anything else, I would be happy with merging it, though I confess that I have not looked into the details.

[kwankyu]

Sure. Done. Moreover

Before the patch:

sage: F = GF(11)
sage: phi = Hom(F, F.extension(2)).an_element()
sage: R.<x,y> = F[]
sage: x.change_ring(phi)
sage: timeit('x.change_ring(phi)')
625 loops, best of 3: 148 μs per loop

After the patch:

sage: F = GF(11)
sage: phi = Hom(F, F.extension(2)).an_element()
sage: R.<x,y> = F[]
sage: x.change_ring(phi)
sage: timeit('x.change_ring(phi)')
625 loops, best of 3: 53 μs per loop

Hence the new change_ring is faster!

[mantepse]

Hence the new change_ring is faster!

I don't understand. Wasn't #36832 about the following?

sage: F = GF(11)
sage: phi = Hom(F,F).an_element()
sage: R.<x,y> = F[]
sage: x.change_ring(phi)

[kwankyu]

Hence the new change_ring is faster!

I don't understand. Wasn't #36832 about the following?

sage: F = GF(11)
sage: phi = Hom(F,F).an_element()
sage: R.<x,y> = F[]
sage: x.change_ring(phi)

Yes. That (which didn't work before) is fixed by this PR. But as a bonus, the PR speeds up already working cases.

[mantepse]

Hence the new change_ring is faster!

I don't understand. Wasn't #36832 about the following?

sage: F = GF(11)
sage: phi = Hom(F,F).an_element()
sage: R.<x,y> = F[]
sage: x.change_ring(phi)

Yes. That (which didn't work before) is fixed by this PR. But as a bonus, the PR speeds up already working cases.

It would be good to make that a doctest, don't you think?

[kwankyu]

Hence the new change_ring is faster!

I don't understand. Wasn't #36832 about the following?

sage: F = GF(11)
sage: phi = Hom(F,F).an_element()
sage: R.<x,y> = F[]
sage: x.change_ring(phi)

Yes. That (which didn't work before) is fixed by this PR. But as a bonus, the PR speeds up already working cases.

It would be good to make that a doctest, don't you think?

Above one? A doctest is already added by ccaa957.

[tornaria]

Notes:

• could you squash together the first two commits? The first one makes a change that the second one changes again. That makes history more difficult to use.
• is it possible to separate the pycodestyle changes from the actual fix?
• could mention Fix change_ring method of multi-variate polynomials #37159 (and/or Random failure in ell_field.py with division_field #36832) in the commit message
• maybe it's reasonable to add a test in ell_field.py that (deterministically) triggers the original issue.
• I'm not sure what happens with the same code in src/sage/rings/polynomial/multi_polynomial_element.py? Is it ever used? In any case, it's either redundant (should be removed) or buggy (also it checks isinstance(R, Morhpism) instead of isinstance(R, Map), this comes also from Change ring to QQbar fails for subschemes #20067 and was fixed in the other two occurrences).

[kwankyu]

Notes:

• could you squash together the first two commits? The first one makes a change that the second one changes again. That makes history more difficult to use.
• is it possible to separate the pycodestyle changes from the actual fix?

Done.

• could mention Fix change_ring method of multi-variate polynomials #37159 (and/or Random failure in ell_field.py with division_field #36832) in the commit message

Sorry but I don't see the need.

• maybe it's reasonable to add a test in ell_field.py that (deterministically) triggers the original issue.

As the failure in ell_field.py is a derivative from one in src/sage/rings/polynomial/multi_polynomial.pyx, for which a test is added, I think a test in ell_field.py is redundant.

• I'm not sure what happens with the same code in src/sage/rings/polynomial/multi_polynomial_element.py? Is it ever used? In any case, it's either redundant (should be removed) or buggy (also it checks isinstance(R, Morhpism) instead of isinstance(R, Map), this comes also from Change ring to QQbar fails for subschemes #20067 and was fixed in the other two occurrences).

Right, it seems redundant. Removed and combined with the one in src/sage/rings/polynomial/multi_polynomial.pyx. Thanks for pointing this out.

[github-actions]

Documentation preview for this PR (built with commit fb1b108; changes) is ready! 🎉

[tornaria]

LGTM

[kwankyu]

Thanks!