Opened 4 years ago

Last modified 4 years ago

#21894 new defect

Do not compare dictionaries which might contain caches

Reported by: saraedum Owned by:
Priority: major Milestone: sage-7.5
Component: pickling Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #19820, #21995 Stopgaps:

Description (last modified by saraedum)

The following is a common pattern in implementations of __cmp__ or __eq__:

sage: search_src("__dict__ == ")
categories/poor_man_map.py:93:        return self.__class__ is other.__class__ and self.__dict__ == other.__dict__
combinat/dlx.py:142:        return self.__dict__ == other.__dict__
combinat/knutson_tao_puzzles.py:586:            return self.__dict__ == other.__dict__
geometry/toric_plotter.py:264:        return type(self) is type(other) and self.__dict__ == other.__dict__
misc/misc.py:1611:        return self.__class__ == other.__class__ and self.__dict__ == other.__dict__
modules/with_basis/morphism.py:334:        return self.__class__ is other.__class__ and parent(self) == parent(other) and self.__dict__ == other.__dict__
modules/with_basis/morphism.py:1474:        return self.__class__ is other.__class__ and self.__dict__ == other.__dict__

This fails as soon as self has a @cached_method:

  • Instances that would be considered equal but have different elements in their cache are not detected as being equal anymore.
  • A restored pickle contains a PickledMethod which is not considered the same as the CachedMethodCaller(NoArgs) in the original object.

Interestingly, almost all these cases, do not implement __ne__ which makes != not the negation of ==. Also, most do not implement __hash__, or implement it incorrectly,

The instances which use this in their implementations of __cmp__ seem to be limited to doctest/. There, such an implementation should be fine:

sage: search_src("cmp\(self.__dict__,")
doctest/control.py:139:        return cmp(self.__dict__,other.__dict__)
doctest/parsing.py:458:        return cmp(self.__dict__, other.__dict__)
doctest/sources.py:152:        return cmp(self.__dict__, other.__dict__)
doctest/util.py:193:        return cmp(self.__dict__, other.__dict__)

Change History (6)

comment:1 Changed 4 years ago by saraedum

  • Description modified (diff)

comment:2 Changed 4 years ago by saraedum

  • Description modified (diff)

comment:3 Changed 4 years ago by saraedum

  • Description modified (diff)

comment:4 Changed 4 years ago by tscrim

See #19820 for the module morphism.

comment:5 Changed 4 years ago by saraedum

  • Dependencies set to #19820

Thanks for pointing that one out.

comment:6 Changed 4 years ago by saraedum

  • Dependencies changed from #19820 to #19820, #21995
Note: See TracTickets for help on using tickets.