Opened 8 years ago
Closed 8 years ago
#5991 closed enhancement (fixed)
[with patch, positive review] Add a standard constructor for dynamic classes
Reported by: | nthiery | Owned by: | nthiery |
---|---|---|---|
Priority: | major | Milestone: | sage-4.2 |
Component: | misc | Keywords: | dynamic classes, pickling, unique representation |
Cc: | sage-combinat, saliola, roed | Merged in: | sage-4.2.alpha0 |
Authors: | Nicolas M. Thiéry | Reviewers: | David Roe |
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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?
Attachments (6)
Change History (24)
comment:1 Changed 8 years ago by
- Summary changed from Add as tandard constructor for dynamic classes to Add a standard constructor for dynamic classes
comment:2 Changed 8 years ago by
- Description modified (diff)
comment:3 Changed 8 years ago by
- Description modified (diff)
comment:4 Changed 8 years ago by
- Summary changed from Add a standard constructor for dynamic classes to [with patch, needs review] Add a standard constructor for dynamic classes
comment:5 Changed 8 years ago by
- Description modified (diff)
comment:6 Changed 8 years ago by
- Status changed from new to assigned
Changed 8 years ago by
comment:7 Changed 8 years ago by
- Summary changed from [with patch, needs review] Add a standard constructor for dynamic classes to [with patch, positive review] Add a standard constructor for dynamic classes
Looks good. Doctests pass.
comment:8 Changed 8 years ago by
- Reviewers set to David Roe
Since this depends on #5985 which does not have a positive review it will have to wait.
comment:9 Changed 8 years ago by
- Summary changed from [with patch, positive review] Add a standard constructor for dynamic classes to [with patch, needs work] Add a standard constructor for dynamic classes
Doctest failures:
sage -t -long devel/sage/sage/structure/dynamic_class.py ********************************************************************** File "/space/boothby/sage-4.0.3/devel/sage-main/sage/structure/dynamic_class.py", line 210: sage: loads(dumps(BarFoo)) Exception raised: Traceback (most recent call last): File "/space/boothby/sage-4.0.3/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/space/boothby/sage-4.0.3/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/space/boothby/sage-4.0.3/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_1[23]>", line 1, in <module> loads(dumps(BarFoo))###line 210: sage: loads(dumps(BarFoo)) File "sage_object.pyx", line 604, in sage.structure.sage_object.dumps (sage/structure/sage_ob$ return comp.compress(cPickle.dumps(obj, protocol=2)) PicklingError: Can't pickle <class '__main__.BarFoo'>: attribute lookup __main__.BarFoo failed ********************************************************************** File "/space/boothby/sage-4.0.3/devel/sage-main/sage/structure/dynamic_class.py", line 224: sage: import sage.misc.cPickle as cPickle Exception raised: Traceback (most recent call last): File "/space/boothby/sage-4.0.3/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/space/boothby/sage-4.0.3/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/space/boothby/sage-4.0.3/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_1[30]>", line 1, in <module> import sage.misc.cPickle as cPickle###line 224: sage: import sage.misc.cPickle as cPickle ImportError: No module named cPickle ********************************************************************** File "/space/boothby/sage-4.0.3/devel/sage-main/sage/structure/dynamic_class.py", line 225: sage: cPickle.loads(cPickle.dumps(FooBar)) == FooBar Exception raised: Traceback (most recent call last): File "/space/boothby/sage-4.0.3/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/space/boothby/sage-4.0.3/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/space/boothby/sage-4.0.3/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_1[31]>", line 1, in <module> cPickle.loads(cPickle.dumps(FooBar)) == FooBar###line 225: sage: cPickle.loads(cPickle.dumps(FooBar)) == FooBar NameError: name 'cPickle' is not defined ********************************************************************** File "/space/boothby/sage-4.0.3/devel/sage-main/sage/structure/dynamic_class.py", line 238: sage: loads(dumps(FooUnique)) is FooUnique Exception raised: Traceback (most recent call last): File "/space/boothby/sage-4.0.3/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/space/boothby/sage-4.0.3/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/space/boothby/sage-4.0.3/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_1[35]>", line 1, in <module> loads(dumps(FooUnique)) is FooUnique###line 238: sage: loads(dumps(FooUnique)) is FooUnique File "sage_object.pyx", line 604, in sage.structure.sage_object.dumps (sage/structure/sage_ob$ return comp.compress(cPickle.dumps(obj, protocol=2)) PicklingError: Can't pickle <class '__main__.Foo'>: it's not the same object as __main__.Foo ********************************************************************** 1 items had failures: 4 of 36 in __main__.example_1 ***Test Failed*** 4 failures.
comment:10 Changed 8 years ago by
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?
Changed 8 years ago by
comment:11 follow-up: ↓ 12 Changed 8 years ago by
- Cc roed added
The two added patches, by David and myself improve introspection. I give a positive review on David's. David, can you double check mine?
Changed 8 years ago by
comment:12 in reply to: ↑ 11 Changed 8 years ago by
Replying to nthiery:
The two added patches, by David and myself improve introspection. I give a positive review on David's. David, can you double check mine?
The updated referee patch by myself adds a copyright header, and fixes a warning in sage -docbuild.
David: please double check!
comment:13 Changed 8 years ago by
- Summary changed from [with patch, needs work] Add a standard constructor for dynamic classes to [with patch, needs review] Add a standard constructor for dynamic classes
David: can you have a quick look at my latest addition, and set a positive review (pending #5985)
comment:14 follow-up: ↓ 15 Changed 8 years ago by
- Summary changed from [with patch, needs review] Add a standard constructor for dynamic classes to [with patch, positive review] Add a standard constructor for dynamic classes
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 in reply to: ↑ 14 Changed 8 years ago by
Replying to roed:
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.
Thanks! All the doctest pass on my machine, but yes, a triple check would be good.
comment:16 Changed 8 years ago by
- Milestone changed from sage-4.1.3 to sage-4.1.2
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 Changed 8 years ago by
NOTE to self -- the change to sageinspect needs to be ported into the separated notebook when this is merged.
Changed 8 years ago by
comment:18 Changed 8 years ago by
- Merged in set to sage-4.2.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
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.
Patch updated for new version of #5120 Please ignore dynamic_class-5991-submitted.2.patch