Opened 11 years ago
Closed 11 years ago
#8250 closed enhancement (fixed)
Extend ClasscallMetaclass to allow for binding behavior
Reported by: | nthiery | Owned by: | nthiery |
---|---|---|---|
Priority: | major | Milestone: | sage-4.3.3 |
Component: | categories | Keywords: | ClassCall, descriptor, __classget__ |
Cc: | sage-combinat | Merged in: | sage-4.3.3.alpha1 |
Authors: | Nicolas M. Thiéry | Reviewers: | Florent Hivert |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
From the documentation:
(using this patch) we show how to implement a nested class Foo.Bar
with a binding behavior, as if it was a method of Foo: namely for
foo
an instance of
Foo
, calling
foo.Bar()
is equivalent to
Foo.Bar(foo)
::
sage: import functools sage: from sage.misc.classcall_metaclass import ClasscallMetaclass sage: class Foo: ... class Bar(object): ... __metaclass__ = ClasscallMetaclass ... @staticmethod ... def __classget__(cls, instance, owner): ... print "calling __classget__" ... if instance is None: ... return cls ... return functools.partial(cls, instance) ... def __init__(self, instance): ... self.instance = instance sage: foo = Foo() sage: bar = foo.Bar() calling __classget__ sage: bar.instance == foo True
This will be used by the upcoming improvements to the functorial constructions in categories
Attachments (2)
Change History (13)
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
- Status changed from new to needs_review
comment:3 follow-up: ↓ 5 Changed 11 years ago by
Hi Nicolas,
They are a few problem with this patch:
- it seems that line 119-125 (in the file) does not belongs to there ! They are after a return. Are they a bad copy paste ?
- I think it would be more natural to add this feature to
NestedClassMetaclass
rather than toClasscallMetaclass
- It took me half an hour to fully understand what's happening. In particular the doc is wrong in saying that
This method implements a binding behavior for ``foo.cls`` by delegating it to ``cls.__classget__(foo)``
indeed cls.classget(foo, Foo)
is called. Can you confirm this ?
Florent
comment:4 Changed 11 years ago by
- Status changed from needs_review to needs_work
comment:5 in reply to: ↑ 3 ; follow-up: ↓ 6 Changed 11 years ago by
Replying to hivert:
Hi Nicolas,
They are a few problem with this patch:
- it seems that line 119-125 (in the file) does not belongs to there ! They are after a return. Are they a bad copy paste ?
Thanks for spotting this; that could explain some trouble I was having right now :-)
- I think it would be more natural to add this feature to
NestedClassMetaclass
rather than toClasscallMetaclass
Let's discuss this over the phone.
- It took me half an hour to fully understand what's happening. In particular the doc is wrong in saying that
This method implements a binding behavior for ``foo.cls`` by delegating it to ``cls.__classget__(foo)``indeed
cls.classget(foo, Foo)
is called. Can you confirm this ?
Oops right. It calls classget as per the descriptor protocol (which includes a 3rd argument).
Please let me know if you already made a patch on top on this one in the queue.
comment:6 in reply to: ↑ 5 Changed 11 years ago by
Replying to nthiery:
- I think it would be more natural to add this feature to
NestedClassMetaclass
rather than toClasscallMetaclass
Let's discuss this over the phone.
Ok.
Please let me know if you already made a patch on top on this one in the queue.
please see trac_8250-classcall-classget-review-fh.patch
in combinat's queue. I'll upload it there as soon as we decided to move to NestedClassMetaclass
or not.
comment:7 Changed 11 years ago by
- Status changed from needs_work to needs_review
comment:8 Changed 11 years ago by
There were still a few problems with the documentation:
- references to methods
__classget__
and__classcall__
instead of__get__
and__call__
; - Missing title for the file;
- Missing use of
NestedClassMetaclass
in the outer class resulting in non standard names for the class. Moreover, the example now demonstrate the correct usage. - I also added these two metaclasses to the reference manual. Nicolas: can you confirm that we want it ?
I uploaded a review patch. Please review :-)
Changed 11 years ago by
comment:9 Changed 11 years ago by
As we spoke on the phone I added a comment and corrected a typo. Ready for review. I'm ok with your patch and if you are ok with mine you can put positive review.
comment:10 Changed 11 years ago by
- Status changed from needs_review to positive_review
comment:11 Changed 11 years ago by
- Merged in set to sage-4.3.3.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
Merged in this order:
This is a check to see if CC to sage-combinat-commits is working. Apologies for the trouble.
Cheers,
Florent