#15247: Introduce a baseclass for singletons
-------------------------------------+-------------------------------------
Reporter: SimonKing | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.3
Component: performance | Resolution:
Keywords: | Merged in:
Authors: Simon King | Reviewers: Marc Mezzarobba,
Report Upstream: N/A | Travis Scrimshaw
Branch: | Work issues:
public/singleton_class-15247 | Commit:
Dependencies: | 0d78737de4a8f364903da0998f02fc9e803c2b0f
| Stopgaps:
-------------------------------------+-------------------------------------
Changes (by tscrim):
* commit: dfbb8bba4a90b1bc30cd33199c9eee9655d8fc95 =>
0d78737de4a8f364903da0998f02fc9e803c2b0f
* branch: u/mmezzarobba/15247-singleton_class-rebased =>
public/singleton_class-15247
* reviewer: Marc Mezzarobba => Marc Mezzarobba, Travis Scrimshaw
Comment:
Perhaps Simon can comment at some point, but here are my thoughts.
Replying to [comment:25 mmezzarobba]:
> * I am a little bit skeptical about tying the singleton behaviour to
comparison by identity if `SingletonClass` is meant to be ''the'' way to
implement singleton classes. Indeed, this makes it virtually unusable for
Elements as long as sage keeps abusing `==` to implement semantic (as
opposed to syntaxic) equality. And singleton classes descending from
Element have at leat one natural use case, namely well-known constants
(infinities, pi...). Could you please comment on this choice?
I would say this is okay as you can always override `__eq__` as necessary,
and python's default is to do equality check by id.
> * I don't like having the same explanations and/or tests repeated in
several docstrings. I tried to remove redundancies in c5acb88a and
949b7b2aa. But feel free to revert these commits if you disagree!
I moved the pickling test to the class-level doc in order to make it more
visible.
> * The same goes for 207f6b72, where I applied `SingletonClass` to other
parents (initially to check how usable it was in practice, but since it's
done...).
> * Finally, I made a few minor changes in d9ca799c. Could you please
check that none of the things I "fixed" was intentional?
I think they were typos.
I've also made some other misc review changes. However I have a question,
should this be in `sage/misc` or in `sage/structure` since it feels more
like a structural piece of Sage? Otherwise I'm happy with the current
state of things (and found similar speedups).
----
New commits:
||[http://git.sagemath.org/sage.git/commit/?id=4ad8e71794f2afe5bae65949426d5670cef4711f
4ad8e71]||{{{Merge branch 'u/mmezzarobba/15247-singleton_class-rebased' of
trac.sagemath.org:sage into public/structure/singleton_class-15247}}}||
||[http://git.sagemath.org/sage.git/commit/?id=0d78737de4a8f364903da0998f02fc9e803c2b0f
0d78737]||{{{Some review changes for #15247.}}}||
--
Ticket URL: <http://trac.sagemath.org/ticket/15247#comment:32>
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.
