Deprecate _populate_coercion_lists_(element_constructor=...)

[jdemeyer]

As an effort to reduce the number of entry points to the "element constructor" (see #23880), get rid of _populate_coercion_lists_(element_constructor=...). Instead, set an Element attribute or implement an _element_constructor_ method.

Unpickling still calls _populate_coercion_lists_(element_constructor=...) and we are not fixing this for now.

CC: @tscrim

Component: coercion

Author: Jeroen Demeyer

Branch/Commit: d7b390b

Reviewer: Travis Scrimshaw

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

[jdemeyer]

Author: Jeroen Demeyer

[jdemeyer]

Branch: u/jdemeyer/deprecate__populate_coercion_lists__element_constructor_____

[jdemeyer]

Commit: d7b390b

[jdemeyer]

New commits:

d7b390b

Deprecate _populate_coercion_lists_(element_constructor=...)

[tscrim]

comment:5

Can you explain why in some cases you did:

_element_constructor_ = integer.Integer

and for others you used Element? Is it for speed reasons (in particular, avoiding a level of indirection), coupled with the fact the elements are extension classes?

Otherwise everything else LGTM (although I would like to see, e.g., BrandtModuleElement._add_ use type(self) instead of BrandtModuleElement as forward-proofing, but I only noticed that because the code simply moved locations and is unrelated to this ticket).

[jdemeyer]

comment:6

Replying to @tscrim:

Can you explain why in some cases you did:

_element_constructor_ = integer.Integer

and for others you used Element?

The reason is that most element classes take the parent as argument. So the default _element_constructor_ method passes the parent to the element class. For ZZ, the parent is constant, so it is not passed in the Integer constructor. So I need a custom _element_constructor_ "method".

Here it is not actually a method but a class used as callable object. I could wrap it in a method, but it works this way so I didn't do that.

[jdemeyer]

comment:7

Replying to @tscrim:

I only noticed that because the code simply moved locations and is unrelated to this ticket).

Indeed. If you do

class BrandtModule_class(AmbientHeckeModule):
    Element = BrandtModuleElement

you need to define BrandtModuleElement before BrandtModule_class.

[tscrim]

comment:8

Replying to @jdemeyer:

Replying to @tscrim:

Can you explain why in some cases you did:

_element_constructor_ = integer.Integer

and for others you used Element?

The reason is that most element classes take the parent as argument. So the default _element_constructor_ method passes the parent to the element class. For ZZ, the parent is constant, so it is not passed in the Integer constructor. So I need a custom _element_constructor_ "method".

Ah, I see. Makes sense. Thanks.

Here it is not actually a method but a class used as callable object. I could wrap it in a method, but it works this way so I didn't do that.

I think what you did is the most sensible way (at least that I can see).

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:9

Replying to @jdemeyer:

Replying to @tscrim:

I only noticed that because the code simply moved locations and is unrelated to this ticket).

Indeed. If you do

class BrandtModule_class(AmbientHeckeModule):
    Element = BrandtModuleElement

you need to define BrandtModuleElement before BrandtModule_class.

Not strictly speaking, as you can do BrandtModule_class.Element = BrandtModuleElement later on down at the module-level, but that is quite ugly IMO.

[vbraun]

Changed branch from u/jdemeyer/deprecate__populate_coercion_lists__element_constructor_____ to d7b390b