#11474: Elliptic curves should be unique parent structures
-------------------------------------+-------------------------------------
Reporter: SimonKing | Owner: cremona
Type: defect | Status: needs_info
Priority: major | Milestone: sage-6.2
Component: elliptic curves | Resolution:
Keywords: unique parent | Merged in:
Authors: Simon King | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/defeo/ticket/11474 | 8cbb73e266096e02efc917545aab36d067b7e063
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by SimonKing):
Replying to [comment:20 pbruin]:
> If I remember correctly, for non-trivial parents it is recommended to
use a `UniqueFactory`, which gives more flexibility than
`UniqueRepresentation`. (I personally also understand `UniqueFactory`
better than the `__classcall__()` magic of Simon's patch, but that may be
simply because I am too familiar with this part of the internals of Sage.)
Maybe Simon or someone else can elaborate on this point?
I don't recall what I did in this patch, and I do not have the bandwith to
look at it right now. So, for now I can only try to explain my general
point of view to factory versus `CachedRepresentation` versus
`UniqueRepresentation` versus `WithEqualityById`.
If one uses a factory, then one can rather cleanly separate the creation
of the cache key from the input data, additional data used to create the
object (without being relevant to the cache) and the creation of the
object to be returned. What it returns is '''not''' a unique parent,
because comparison and hash is (and can not) be taken care of by the
factory. Drawbacks:
- One needs to write the factory code. At least in trivial cases,
`CachedRepresentation` involves less manual work.
- Suppose you used to have a factory for some parents, but later decide to
remove the factory. If you remove the factory from Sage, then old pickles
will break, since they rely on the factory being available. This is in
contrast to `CachedRepresentation`, where you just need that the class
name is preserved: If you remove `CachedRepresentation` from the base
classes, then old pickles can still be opened.
- I think it is not nice that one has to do `MyFancyStuff(args)` to create
an object, but then the resulting object is not an instance of
`MyFancyStuff`. But that might be a matter of taste.
`CachedRepresentation` is essentially equivalent to using a factory. In
particular, it does '''not''' result in unique parents. Drawbacks:
- It can not be used on extension classes, since a metaclass is involved
(which is not supported by Cython).
- It does not cleanly separate creation of the cache, additional arguments
and object creation. You can write `__classcall(_private)__` so that it
does exactly the same as a factory, it can look up stuff in a database,
and so on. But the code is less clean and thus more error prone.
So, at this point, we have some kind of factory, but no uniqueness of
parents. In order to add unique parent behaviour, one can simply add
`WithEqualityById` to the list of base classes (and make sure that hash
and comparison will not be overridden in the sub-class).
`UniqueRepresentation` simply is a ready-made combination of
`CachedRepresentation` and `WithEqualityById`, there is nothing more to
it.
I hope that these remarks help to assess whether it would be better to use
a factory PLUS `WithEqualityById` on elliptic curves, or "simply" use
`UniqueRepresentation`.
--
Ticket URL: <http://trac.sagemath.org/ticket/11474#comment:29>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
