-
-
Notifications
You must be signed in to change notification settings - Fork 560
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
Add a standard constructor for dynamic classes #5991
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
comment:4
Patch updated for new version of #5120 |
This comment has been minimized.
This comment has been minimized.
Attachment: dynamic_class-5991-submitted.patch.gz |
comment:7
Looks good. Doctests pass. |
Author: Nicolas Thiery |
comment:8
Since this depends on #5985 which does not have a positive review it will have to wait. |
Reviewer: David Roe |
comment:9
Doctest failures:
|
comment:10
Yeah: it depends on #5985, but otherwise is ready. Should there be a new field in the trac server for listing systematically the other tickets the ticket depends on? |
comment:11
Attachment: trac_5991-dynamic_class-referee-dr.patch.gz The two added patches, by David and myself improve introspection. |
Changed author from Nicolas Thiery to Nicolas M. Thiéry |
comment:12
Attachment: trac_5991-dynamic_class-referee-nt.patch.gz Replying to @nthiery:
The updated referee patch by myself adds a copyright header, and fixes a warning in sage -docbuild. David: please double check! |
Attachment: dynamic_class-reduction-nt.patch.gz Fix reduction after David's referee patch |
Fold of all the patches above. Apply only this one |
comment:13
Attachment: trac_5991_dynamic_class-nt.patch.gz David: can you have a quick look at my latest addition, and set a positive review (pending #5985) |
comment:14
So, I approve the changes. I don't have time right now to run doctests though: the release manager (or someone) should make sure to do so. |
comment:15
Replying to @roed314:
Thanks! All the doctest pass on my machine, but yes, a triple check would be good. |
comment:16
If #5985 is ready on time to get integrated in 4.1.2, it would be great to have this on go in too. |
comment:17
NOTE to self -- the change to sageinspect needs to be ported into the separated notebook when this is merged. |
Attachment: trac_5991_dynamic_class-nt.2.patch.gz |
Merged: sage-4.2.alpha0 |
comment:18
I had to make the change to the way cPickle is imported as in #5985. I'll make a new ticket for sageinspect in the separated notebook. |
This patch implements sage.structure.dynamic_class.dynamic_class, for constructing dynamically new python classes. The constructed classes can be pickled, and have unique representation.
The patch includes a discussion on the relevance of dynamic classes for Sage.
Depends on #5985 for pickling and #5120.
Used by the upcoming category framework #5891, (and sage-words?)
Issue: is sage.structure.dynamic_class.dynamic_class the natural location for this?
CC: @sagetrac-sage-combinat @saliola @roed314
Component: misc
Keywords: dynamic classes, unique representation
Author: Nicolas M. Thiéry
Reviewer: David Roe
Merged: sage-4.2.alpha0
Issue created by migration from https://trac.sagemath.org/ticket/5991
The text was updated successfully, but these errors were encountered: