UniqueRepresentation MRO Question

109 views
Skip to first unread message

Travis Scrimshaw

unread,
Mar 13, 2024, 11:58:49 AM3/13/24
to sage-devel
Hi everyone,
   On https://github.com/sagemath/sage/pull/37128, we find it useful to swap the order of the base classes for UniqueRepresentation. Thus, it becomes

class UniqueRepresentation(WithEqualityById, CachedRepresentation):

I believe this is very reasonable since we want to have objects' (typically parents, but not limited to them) comparison and hashing be first in the MRO. In particular, this is important if we have the following setup:

class Foo(CachedRepresentation):
    pass

class Bar(UniqueRepresentation, Foo):
    pass

That way Bar gets the main benefits of being a UR (fast equality and hashing). With the current way, there is no way (that I know) to simply have the corresponding methods at the Cython level (at best, there is a Python redirect that has to be manually implemented) since WithEqualityById is a Cython cdef class.

However, we then have a problem with unpickling, but I suspect it is a bigger issue. From combinat/root_system/cartan_type.py, the last line fails with an incompatible class order:

            sage: pg_CartanType_simple_finite = unpickle_global('sage.combinat.root_system.cartan_type', 'CartanType_simple_finite')
            sage: si1 = unpickle_newobj(pg_CartanType_simple_finite, ())
            sage: from sage.misc.fpickle import unpickleModule
            sage: pg_make_integer = unpickle_global('sage.rings.integer', 'make_integer')
            sage: si2 = pg_make_integer('4')
            sage: unpickle_build(si1, {'tools':unpickleModule('sage.combinat.root_system.type_A'), 't':['A', si2], 'letter':'A', 'n':si2})

Mainly, the last line does a self.__class__ = UR.__class__, where UR is an instance of a UniqueRepresentation. Granted, this is a very old pickle, but I am worried it isn't a problem with this specific pickle.

This is beyond my knowledge of Cython. Any thoughts or advice about this issue would be appreciated.

Thank you,
Travis

enriqu...@gmail.com

unread,
Mar 17, 2024, 12:15:18 PM3/17/24
to sage-devel
Is there any sense to define a new class of UniqueRepresentation with the same characteristics and  other name with the switch of order?  

Travis Scrimshaw

unread,
Mar 20, 2024, 10:24:30 AM3/20/24
to sage-devel
Dear Enrique,
   No, that would just duplicate functionality and make things more complicated.

To Cython and/or Python experts,
   It would be great if someone could give some comments about this. This feels like a very natural thing to do, but it might lead to a very subtle breakage in the wild. If there is anything additional I can provide, please just ask.

Thank you,
Travis
Reply all
Reply to author
Forward
0 new messages