wrong inverse action when using ConstructionFunctor.coercion_reversed

[dkrenn]

sage: Q.<y> = SR.subring(no_variables=True)[[]]
sage: (y / 1).parent()
Univariate Polynomial Ring in x over Symbolic Ring

but the result should be

Univariate Polynomial Ring in x over Symbolic Constants Subring

since there is a coercion from the integer ring to the symbolc constants subring.

This is because the attrirute coercion_reversed of ConstructionFunctor is not taken into account.

[dkrenn]

BTW: For some reason it works (partially) for polynomial rings:

     sage: R.<x> = SR.subring(no_variables=True)[]
     sage: (x / 1).parent()
     Univariate Polynomial Ring in x over Symbolic Constants Subring

but

     sage: cm = sage.structure.element.get_coercion_model()
     sage: cm.explain(x, 1, operator.div)
     Action discovered.
        Right inverse action by Symbolic Ring on Univariate Polynomial Ring in x over Symbolic Constants Subring
        with precomposition on right by Conversion map:
          From: Integer Ring
          To:   Symbolic Ring
    Result lives in Univariate Polynomial Ring in x over Symbolic Ring
    Univariate Polynomial Ring in x over Symbolic Ring

At one point this should be investigated, but probably on a different ticket.

[dkrenn]

Branch should fix this; still need to run a make ptestlong; will do this soon...

---

Last 10 new commits:

​fb2885c	subring in index.rst
​eab9513	simplify a doctest
​23352b2	change ValueError to TypeError to make everything work with SR as it should
​2d8b7c4	typo in docstring
​15ec40e	docstring of SR.subring
​7938d95	module description of subring
​852959a	rename only_constants --> no_variables
​e4837e9	correct parent of result of an_element
​6e5a098	Merge branch 'u/dkrenn/symbolic-subring' of trac.sagemath.org:sage into coerce/inverse-action
​5638459	#19521: use coercion_reversed in InverseAction

[dkrenn]

Replying to dkrenn:

Branch should fix this; still need to run a make ptestlong; will do this soon...

Passed. So I set it to needs review.

[behackl]

Hi! I've merged the (positively reviewed) dependency into this branch and reviewed the code in this ticket.

I think I understand your fix as well as the strategy behind it, and I am willing to give this a positive_review (i.e. everything LGTM). However, I'd rather be sure:

In principle, all that happens is that you order the functors and parents from smallest to largest (in the sense of coercion) and iterate over this reordered tower such that actually the smallest parent with the required property is found?

For example, (using your notation from the comment), if we had a tower A -> B <- C <- D -> E, then the old code would iterate over [A, B, C, D, E, F], and in your fixed version we iterate over [A, D, C, B, E]?

Also, I'd like to give the patchbots a chance to test this---or, at least, run make ptestlong myself.

---

New commits:

​ff99c7f	Trac #19259: change to has_valid_variable
​581f315	Trac #19259: check validity of variables
​c9b2428	improve language
​05dc834	misc. changes, indentation, line breaks
​8cc884a	fix merge of functors
​aeae8f3	Merge tag '7.0' into symbolics/symbolic-subring
​c4a0e22	merge accepting and rejecting functors in all cases
​d376b10	revert changes to merge of functors
​867d6d3	Merge branch 'u/behackl/symbolics/symbolic-subring' of git://trac.sagemath.org/sage into coercion/inverse-action

[dkrenn]

Replying to behackl:

Hi! I've merged the (positively reviewed) dependency into this branch and reviewed the code in this ticket.

Thanks.

In principle, all that happens is that you order the functors and parents from smallest to largest (in the sense of coercion) and iterate over this reordered tower such that actually the smallest parent with the required property is found?

For example, (using your notation from the comment), if we had a tower A -> B <- C <- D -> E, then the old code would iterate over [A, B, C, D, E, F], and in your fixed version we iterate over [A, D, C, B, E]?

Correct.

Also, I'd like to give the patchbots a chance to test this---or, at least, run make ptestlong myself.

Ok.

[behackl]

... alright. Passes ptestlong and does what it should -> positive_review.