modernize coercion in monsky_washnitzer.py

[fchapoton]

to help for #33497

CC: @videlec @tscrim

Component: refactoring

Author: Frédéric Chapoton

Branch/Commit: 70362ae

Reviewer: Travis Scrimshaw

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

[sagetrac-git]

Changed commit from c808d47 to 5abbdf2

[sagetrac-git]

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

5abbdf2

adding doc

[fchapoton]

comment:3

there remains one little failing doctest..

[tscrim]

comment:4

That doctest will not pass in the current implementation (and I am not sure it should) as B = Z[‘t’] and R._poly_ring == Z[‘T’], where Z = Integers(125) and we do not allow coercion between polynomial rings with different variable names IIRC. I don’t see an equivalent doctest in the old code. Why should this doctest work? I would have expected it to be from Z.

Some comments for possible optimizations for general modernization (feel free to push them to a followup ticket, which I can do):

• In _mul_, it will never be the case that the first check is true by the coercion framework. I also believe the todo with the recast is also a no-op and can be removed once the cast is done in the parent.
• If a matrix is hard-coded and then an inverse taken, why don’t we just hard-code the inverse?
• __repr__ to _repr_ in the parent.
• gens() should return a tuple and it can be cached. Although, IIRC, this is taken care of automatically if gen() is implemented instead. (It’s better to use zero() and one() there as well.)
• Why not make create_element() an alias of _element_constructor_()?
• @cached_method the one().
• I know I would appreciate having those long output lines broken over multiple lines, but that is very cosmetic.
• Cythonize the element.
• Use _poly_ring instead of poly_ring() in the element __init__ check.
• I don’ think we need the Python2 compatibility __nonzero__ = __bool__ alias.
• Indentation levels on various INPUT: bullet points.

[sagetrac-git]

Changed commit from 5abbdf2 to c2c5114

[sagetrac-git]

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

c2c5114

various changes in monsky after reviewer's suggestions

[fchapoton]

comment:6

Concerning the coercion test, it seems strange to me that the variable "T" is hardcoded.

About your suggestions, I did what I could. Maybe one can keep the rest for another ticket.

[sagetrac-git]

Changed commit from c2c5114 to cbf7db6

[sagetrac-git]

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

cbf7db6

one little detail

[tscrim]

comment:8

Thank you.

Having the variable T being hard coded is mildly strange, but that variable is unrelated to the variable for the univariate polynomial that defines the quotient. In the examples, it gets changed from t to x. Perhaps then the thing to do is to check that either it coerces into the base ring (of _poly_ring) or it is a univariate polynomial ring that whose base ring coerces into the base ring?

[sagetrac-git]

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

6cae3da

fix doctests

[sagetrac-git]

Changed commit from cbf7db6 to 6cae3da

[sagetrac-git]

Changed commit from 6cae3da to 70362ae

[sagetrac-git]

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

70362ae

details in doc

[fchapoton]

comment:11

so we have a green bot. Good enough ?

[tscrim]

comment:12

Yes, thank you.

[tscrim]

Reviewer: Travis Scrimshaw

[vbraun]

Changed branch from public/monsky_coercion to 70362ae