Skip to content

Commit

Permalink
sagemathgh-38203: Change inheritance order in UniqueRepresentation
Browse files Browse the repository at this point in the history
    
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Currently in `UniqueRepresentation`, the MRO is constructed by
```class UniqueRepresentation(CachedRepresentation,
WithEqualityById):```
However, in order to maximize the utility of the `WithEqualityById`
class, in particular with subclasses of `UniqueRepresentation`, we
should place it *first* in the MRO. This is useful in sagemath#37128, where this
resolves MRO resolution issues and not having to reimplement (or set
aliases) the behavior of `WithEqualityById`.

However, this change, while seemingly innocuous, does cause a problem
with one doctest dealing with very old pickles (Sage version <= 4.0) in
`combinat/root_system/cartan_type.py` concerning `class
CartanType_simple_finite:`. Currently, it is unclear why this change is
resulting in the error:
```
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})
Traceback (most recent call last):
  File "/sage/src/sage/doctest/forker.py", line 715, in _run
    self.compile_and_execute(example, compiler, test.globs)
  File "/sage/src/sage/doctest/forker.py", line 1147, in
compile_and_execute
    exec(compiled, globs)
  File "<doctest sage.combinat.root_system.cartan_type.CartanType_simple
_finite.__setstate__[5]>", line 1, in <module>
    unpickle_build(si1,
{'tools':unpickleModule('sage.combinat.root_system.type_A'), 't':['A',
si2], 'letter':'A', 'n':si2})
  File "/sage/src/sage/misc/explain_pickle.py", line 2442, in
unpickle_build
    setstate(state)
  File "/sage/src/sage/combinat/root_system/cartan_type.py", line 3106,
in __setstate__
    self.__class__ = T.__class__
TypeError: __class__ assignment: 'CartanType' object layout differs from
'CartanType_simple_finite'
```
However, since these are very old pickles and we cannot reproduce the
failure otherwise, we feel that it is prudent to drop support for this
and remove the corresponding class.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38203
Reported by: Enrique Manuel Artal Bartolo
Reviewer(s): Travis Scrimshaw
  • Loading branch information
Release Manager committed Jul 24, 2024
2 parents 79c047c + ad01e1d commit e53a602
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 39 deletions.
2 changes: 1 addition & 1 deletion src/sage/categories/category_singleton.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,9 @@ class Category_singleton(Category):
<class 'sage.categories.category_singleton.Category_singleton'>,
<class 'sage.categories.category.Category'>,
<class 'sage.structure.unique_representation.UniqueRepresentation'>,
<class 'sage.misc.fast_methods.WithEqualityById'>,
<class 'sage.structure.unique_representation.CachedRepresentation'>,
<class 'sage.structure.unique_representation.WithPicklingByInitArgs'>,
<class 'sage.misc.fast_methods.WithEqualityById'>,
<class 'sage.structure.sage_object.SageObject'>,
<class '__main__.R.subcategory_class'>,
<class 'sage.categories.sets_cat.Sets.subcategory_class'>,
Expand Down
2 changes: 1 addition & 1 deletion src/sage/categories/sets_cat.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,9 @@ class Sets(Category_singleton):
<class 'sage.categories.examples.sets_cat.PrimeNumbers_Inherits'>
<class 'sage.categories.examples.sets_cat.PrimeNumbers_Abstract'>
<class 'sage.structure.unique_representation.UniqueRepresentation'>
<class 'sage.misc.fast_methods.WithEqualityById'>
<class 'sage.structure.unique_representation.CachedRepresentation'>
<class 'sage.structure.unique_representation.WithPicklingByInitArgs'>
<class 'sage.misc.fast_methods.WithEqualityById'>
<class 'sage.structure.parent.Parent'>
<class 'sage.structure.category_object.CategoryObject'>
<class 'sage.structure.sage_object.SageObject'>
Expand Down
36 changes: 0 additions & 36 deletions src/sage/combinat/root_system/cartan_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -3069,39 +3069,3 @@ def __getitem__(self, i):
raise IndexError("index out of range")

options = CartanType.options

##############################################################################
# For backward compatibility


class CartanType_simple_finite:
def __setstate__(self, dict):
"""
Implements the unpickling of Cartan types pickled by Sage <= 4.0.
EXAMPLES:
This is the pickle for CartanType(["A", 4])::
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})
sage: si1
['A', 4]
sage: si1.dynkin_diagram() # needs sage.graphs
O---O---O---O
1 2 3 4
A4
This is quite hacky; in particular unique representation is not preserved::
sage: si1 == CartanType(["A", 4]) # todo: not implemented
True
"""
T = CartanType([dict['letter'], dict['n']])
self.__class__ = T.__class__
self.__dict__ = T.__dict__
2 changes: 1 addition & 1 deletion src/sage/structure/unique_representation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,7 @@ def _clear_cache_(cls):
del cache[k]


class UniqueRepresentation(CachedRepresentation, WithEqualityById):
class UniqueRepresentation(WithEqualityById, CachedRepresentation):
r"""
Classes derived from ``UniqueRepresentation`` inherit a unique
representation behavior for their instances.
Expand Down

0 comments on commit e53a602

Please sign in to comment.