Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not compare dictionaries which might contain caches #21894

Open
saraedum opened this issue Nov 18, 2016 · 7 comments
Open

Do not compare dictionaries which might contain caches #21894

saraedum opened this issue Nov 18, 2016 · 7 comments

Comments

@saraedum
Copy link
Member

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__)

Depends on #19820
Depends on #21995

Component: pickling

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

@saraedum saraedum added this to the sage-7.5 milestone Nov 18, 2016
@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Nov 18, 2016

comment:4

See #19820 for the module morphism.

@saraedum
Copy link
Member Author

comment:5

Thanks for pointing that one out.

@saraedum
Copy link
Member Author

Dependencies: #19820

@saraedum
Copy link
Member Author

Changed dependencies from #19820 to #19820, #21995

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants