Opened 10 years ago

Closed 10 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 10 years ago.
trac_5991-dynamic_class-referee-dr.patch (3.0 KB) - added by nthiery 10 years ago.
trac_5991-dynamic_class-referee-nt.patch (5.2 KB) - added by nthiery 10 years ago.
dynamic_class-reduction-nt.patch (3.7 KB) - added by nthiery 10 years ago.
Fix reduction after David's referee patch
trac_5991_dynamic_class-nt.patch (16.4 KB) - added by nthiery 10 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 10 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 Changed 10 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 10 years ago by nthiery

  • Description modified (diff)

comment:3 Changed 10 years ago by nthiery

  • Description modified (diff)

comment:4 Changed 10 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 10 years ago by robertwb

  • Description modified (diff)

comment:6 Changed 10 years ago by nthiery

  • Status changed from new to assigned

Changed 10 years ago by nthiery

comment:7 Changed 10 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 10 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 10 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 10 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 10 years ago by nthiery

comment:11 follow-up: Changed 10 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 10 years ago by nthiery

comment:12 in reply to: ↑ 11 Changed 10 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 10 years ago by nthiery

Fix reduction after David's referee patch

Changed 10 years ago by nthiery

Fold of all the patches above. Apply only this one

comment:13 Changed 10 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 10 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 10 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 10 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 10 years ago by was

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

Changed 10 years ago by mhansen

comment:18 Changed 10 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.