Add classcall setter in ClasscallMetaclass

[nthiery]

The optimization of ClasscallMetaclass in #12808 prevents dynamically changing the __classcall__ method of a class. This followup adds an appropriate setter to enable this feature.

CC: @hivert @simon-king-jena

Component: misc

Keywords: classcall UniqueRepresentation

Author: Nicolas M. Thiéry

Reviewer: Florent Hivert

Merged: sage-5.10.beta1

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

[simon-king-jena]

comment:1

Is it ready for review, yet?

[nthiery]

comment:2

Replying to @simon-king-jena:

Is it ready for review, yet?

I was just hesitant whether we wanted to aim (in the long run) for one setter for each special method (set_classcall, set_classget, ...), or a single setter for all of them (and in that case what name we want for this setter).

Other than that, yes, needs review!

[hivert]

comment:3

Hi Nicolas,

Thanks for this one. I've a request: You should describe the input of
_set_classcall__ saying that the function must accept the class itself
at first argument. I'll try to post a review patch.

I'm also w little worried about the interface: Alternatively we could either

• make the attribute classcall public (maybe by renaming it with
  underscores (I mistyped underscare at first ;-)).

• or use a property to have a real setter allowing to a nicer syntax. I'm
  pretty sure Cython allows ot too.

Florent

[hivert]

comment:4

Replying to @hivert:

I'm also w little worried about the interface: Alternatively we could either

• make the attribute classcall public (maybe by renaming it with
  underscores (I mistyped underscare at first ;-)).

• or use a property to have a real setter allowing to a nicer syntax. I'm
  pretty sure Cython allows ot too.

Nicolas: any idea about that ?

Florent

[nthiery]

comment:5

Replying to @hivert:

Replying to @hivert:

I'm also w little worried about the interface: Alternatively we could either

• make the attribute classcall public (maybe by renaming it with
  underscores (I mistyped underscare at first ;-)).

• or use a property to have a real setter allowing to a nicer syntax. I'm
  pretty sure Cython allows ot too.

Nicolas: any idea about that ?

Oops, I let that ticket die, whereas I need it soon in Sage for the
functorial constructions. Thanks for the reminder!

If the property approach can be made to work, it sounds like the one
that encapsulates best the details of the caching of the resolution of
classcall. So I vote for it. Does it look feasible?

Cheers,
Nicolas

[hivert]

comment:6

Replying to @nthiery:

Oops, I let that ticket die, whereas I need it soon in Sage for the
functorial constructions. Thanks for the reminder!

If the property approach can be made to work, it sounds like the one
that encapsulates best the details of the caching of the resolution of
classcall. So I vote for it. Does it look feasible?

See: http://docs.cython.org/src/userguide/extension_types.html#properties

We just need to check that we have a sufficiently recent Cython in Sage and
that this doesn't induce some unacceptable overhead.

Cheers,

Florent

[nthiery]

comment:7

Replying to @hivert:

Replying to @nthiery:

Oops, I let that ticket die, whereas I need it soon in Sage for the
functorial constructions. Thanks for the reminder!

If the property approach can be made to work, it sounds like the one
that encapsulates best the details of the caching of the resolution of
classcall. So I vote for it. Does it look feasible?

See: http://docs.cython.org/src/userguide/extension_types.html#properties

We just need to check that we have a sufficiently recent Cython in Sage and
that this doesn't induce some unacceptable overhead.

I just looked at this, and it seems we can't make this approach work:
Indeed, even if we define __classcall__ as a property in
ClasscallMetaclass, any class C which will implement its own
__classcall__ will overwrite the property. But then,
C.classcall = ... won't work ...

So at this point I'd go with just the _set_classcall approach, unless you see a way out.

[nthiery]

comment:8

Updated patch which uses cls rather than self for the first argument of _set_classcall. The interface is not perfect, but that's an internal method and we can change this later on if we get a better idea.

[hivert]

comment:9

Attachment: trac_12894-classcall_setter-nt.patch.gz

[jdemeyer]

Merged: sage-5.10.beta1