Changes between Initial Version and Version 12 of Ticket #14054


Ignore:
Timestamp:
02/19/13 16:00:35 (7 years ago)
Author:
SimonKing
Comment:

The patch should now be ready for review.

Two questions that I'd like to have addressed:

  1. I introduce provide_hash_by_id, but I don't use it. Shall it be deleted?
  2. Is it enough to override the rich comparison methods? Currently, one can have hash(a)!=hash(b) but cmp(a,b)==0. Does this violate the contract of hash functions? Or is it enough that hash(a)!=hash(b) implies a!=b?

Pathbot:

Apply trac14054_fast_methods.patch

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #14054

    • Property Status changed from new to needs_review
    • Property Cc nthiery added
    • Property Reviewers changed from to Travis Scrimshaw
  • Ticket #14054 – Description

    initial v12  
    1 `UniqueRepresentation` provides a comfortable way to create unique parent structures, and automatically provides a hash and certain comparison methods. Problem: It relies on a metaclass, namely `ClasscallMetaclass` and thus has to be a Python class. That's bad for speed.
     1`UniqueRepresentation` provides a comfortable way to create unique parent structures, and automatically provides a hash and certain comparison methods.
    22
    3 Here, I suggest to create a new cdef class `UniqueRepresentation_c` that provides hash and comparison, and let `UniqueRepresentation` inherit from it, just adding the classcall method.
     3Problems:
    44
    5 The problem is that `UniqueRepresentation` relies on cached_method, and this decorator had problems to work on methods defined in Cython with varargs and keywords arguments. That is fixed in #14017, which is thus a dependency.
     5- It relies on a metaclass, namely `ClasscallMetaclass` and thus has to be a Python class. That's bad for speed.
     6- It combines two features, namely a cache and unique instance behaviour. But in many some cases we want a cache so that two distinct instances can still be equal.
     7
     8Here, I suggest to
     9
     101. separate the two features, by creating a new class `sage.unique_representation.CachedRepresentation` as one base of `UniqueRepresentation`.
     112. create a new cdef class `sage.misc.fast_methods.WithRichCmpById`, that provides hash and rich comparison, as expected for unique instance behaviour.
     12
     13__Apply__
     14
     15- [attachment:trac14054_fast_methods.patch]