Opened 9 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 robertwb)

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)

dynamic_class-5991-submitted.patch (13.1 KB) - added by nthiery 9 years ago.
trac_5991-dynamic_class-referee-dr.patch (3.0 KB) - added by nthiery 8 years ago.
trac_5991-dynamic_class-referee-nt.patch (5.2 KB) - added by nthiery 8 years ago.
dynamic_class-reduction-nt.patch (3.7 KB) - added by nthiery 8 years ago.
Fix reduction after David's referee patch
trac_5991_dynamic_class-nt.patch (16.4 KB) - added by nthiery 8 years ago.
Fold of all the patches above. Apply only this one
trac_5991_dynamic_class-nt.2.patch (16.3 KB) - added by mhansen 8 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 Changed 9 years ago by nthiery

  • Summary changed from Add as tandard constructor for dynamic classes to Add a standard constructor for dynamic classes

comment:2 Changed 9 years ago by nthiery

  • Description modified (diff)

comment:3 Changed 9 years ago by nthiery

  • Description modified (diff)

comment:4 Changed 9 years ago by nthiery

  • Summary changed from Add a standard constructor for dynamic classes to [with patch, needs review] Add a standard constructor for dynamic classes

Patch updated for new version of #5120 Please ignore dynamic_class-5991-submitted.2.patch

comment:5 Changed 9 years ago by robertwb

  • Description modified (diff)

comment:6 Changed 9 years ago by nthiery

  • Status changed from new to assigned

Changed 9 years ago by nthiery

comment:7 Changed 9 years ago by roed

  • 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 ncalexan

  • Authors set to Nicolas Thiery
  • 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 boothby

  • 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 nthiery

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 nthiery

comment:11 follow-up: Changed 8 years ago by nthiery

  • Authors changed from Nicolas Thiery to Nicolas M. Thiéry
  • 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 nthiery

comment:12 in reply to: ↑ 11 Changed 8 years ago by nthiery

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!

Changed 8 years ago by nthiery

Fix reduction after David's referee patch

Changed 8 years ago by nthiery

Fold of all the patches above. Apply only this one

comment:13 Changed 8 years ago by nthiery

  • 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: Changed 8 years ago by roed

  • 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 nthiery

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 nthiery

  • 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 was

NOTE to self -- the change to sageinspect needs to be ported into the separated notebook when this is merged.

Changed 8 years ago by mhansen

comment:18 Changed 8 years ago by mhansen

  • 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.

Note: See TracTickets for help on using tickets.