classification
Title: Unbinding of methods
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: duplicate
Dependencies: Superseder: Implement PEP 3154 (pickle protocol 4)
View: 17810
Assigned To: Nosy List: alexandre.vassalotti, asvetlov, daniel.urban, loewis, meador.inge, mstefanro, ncoghlan, rhettinger, sbt, yselivanov
Priority: normal Keywords: patch

Created on 2012-07-19 17:29 by mstefanro, last changed 2013-11-24 04:36 by alexandre.vassalotti. This issue is now closed.

Files
File name Uploaded Description Edit
func.patch mstefanro, 2012-07-19 17:28 Add __func__, some types and inspect stuff, functools.unbind
unbind_test.patch mstefanro, 2012-07-19 19:07 functools.unbind tests
Messages (11)
msg165845 - (view) Author: Stefan Mihaila (mstefanro) * Date: 2012-07-19 17:28
In order to implement pickling of instance methods, a means of separating
the object and the unbound method is necessary.

This is easily done for Python methods (f.__self__ and f.__func__),
but not all of builtins support __func__. Moreover, there currently
appears to be no good way to distinguish functions from bound methods.

As a first step in solving this issue, I have attached a patch which:
1) adds __func__ for all function types
2) adds a few new definitions in the types module (AllFunctionTypes etc.)
3) adds isanyfunction(), isanyboundfunction(), isanyunboundfunction() in
  inspect (admittedly these are bad names)
4) functools.unbind

In case applying this patch is being considered, serious review is necessary,
as I'm not knowledgeable of cpython internals.
msg165846 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-07-19 18:09
Can you push patch in form available for review via Rietveld?
msg165855 - (view) Author: Stefan Mihaila (mstefanro) * Date: 2012-07-19 19:07
Yes, the patch is at http://codereview.appspot.com/6425052/
The code there also contains some tests I've written for functools.unbind.
msg165873 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-07-19 20:00
Looks like PyCFunction_NewEx is part of Stable API.
If I'm right you have to make stub for this one as simple trampoline to new PyCFunction_NewExEx implementation.

Martin, please confirm.
msg165874 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-07-19 20:23
Andrew is right: PyCFunction_NewEx must stay, and must continue to get the same parameters as it currently does. This not only applies to extensions already built, but also to extensions that are built against the new header files: they still need to run under old Python releases (if they only use the stable ABI).
msg165881 - (view) Author: Stefan Mihaila (mstefanro) * Date: 2012-07-19 21:50
Doesn't the definition I've added at the end of methodobject.c suffice? (http://codereview.appspot.com/6425052/patch/1/10) Or should the macro be removed altogether?
msg165907 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-07-20 09:49
Stefan, you right.
A bit hairy idiom from my perspective, but it works.
Looks like this way used only for PyCFunction_New, all other code follows standard schema with trampoline.
msg165948 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-07-20 17:08
Can't you unbind without any changes to the C code by doing

    def unbind(f):
        if hasattr(f, '__func__'):
            return f.__func__
        self = getattr(f, '__self__', None)
        if self is not None and not isinstance(self, types.ModuleType):
            return getattr(type(f.__self__), f.__name__)
        raise TypeError('not a bound method')

Also, I am not convinced that it is a good idea to return f if f is already "unbound".  In practice I think you will always need to treat the bound and the unbound cases differently.
msg166136 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-07-22 14:13
Stefan, I've fixed refleak found by you in #15404. Thanks.
msg166142 - (view) Author: Stefan Mihaila (mstefanro) * Date: 2012-07-22 15:19
Richard, yes, I think that would work, I didn't think of using f.__self__'s type.
You might want to replace
  if self is not None and not isinstance(self, types.ModuleType):
with
  if self is not None and not isinstance(self, types.ModuleType) \
                      and not isinstance(self, type):
to correctly raise an exception when called on a classmethod too.
msg166144 - (view) Author: Stefan Mihaila (mstefanro) * Date: 2012-07-22 15:42
Andrew, thanks for creating a separate issue (the refleak was very rare and I thought I'd put it in the same place, but now I realize it was a bad idea).

Richard, actually, the isinstance(self, type) check I mentioned earlier would have to be before the hastattr(f, '__func__') check, because Python classmethods provide a __func__ too:

    def unbind(f):
        self = getattr(f, '__self__', None)
        if self is not None and not isinstance(self, types.ModuleType) \
                            and not isinstance(self, type):
            if hasattr(f, '__func__'):
                return f.__func__
            return getattr(type(f.__self__), f.__name__)
        raise TypeError('not a bound method')

Anyway, I'm not convinced this is worth adding anymore. As Antoine Pitrou suggested on the ml, it would probably be a better idea if I implemented __reduce__ for builtin methods as well as Python methods rather than having a separate opcode for pickling methods.
History
Date User Action Args
2013-11-24 04:36:50alexandre.vassalottisetstatus: open -> closed
superseder: Implement PEP 3154 (pickle protocol 4)
resolution: duplicate
2013-04-21 06:57:47alexandre.vassalottilinkissue17810 dependencies
2013-04-21 06:56:16alexandre.vassalottiunlinkissue15642 dependencies
2012-08-17 21:40:39alexandre.vassalottilinkissue15642 dependencies
2012-07-22 15:42:55mstefanrosetmessages: + msg166144
2012-07-22 15:19:20mstefanrosetmessages: + msg166142
2012-07-22 14:13:05asvetlovsetmessages: + msg166136
2012-07-20 17:08:19sbtsetnosy: + sbt
messages: + msg165948
2012-07-20 09:49:01asvetlovsetmessages: + msg165907
2012-07-19 21:50:50mstefanrosetmessages: + msg165881
2012-07-19 20:23:24loewissetmessages: + msg165874
2012-07-19 20:00:55asvetlovsetnosy: + loewis
messages: + msg165873
2012-07-19 19:45:38daniel.urbansetnosy: + daniel.urban
2012-07-19 19:07:23mstefanrosetfiles: + unbind_test.patch

messages: + msg165855
2012-07-19 18:11:32yselivanovsetnosy: + yselivanov
2012-07-19 18:09:31asvetlovsetmessages: + msg165846
2012-07-19 18:06:50asvetlovsetnosy: + asvetlov
2012-07-19 17:54:27meador.ingesetnosy: + meador.inge

stage: patch review
2012-07-19 17:35:32mstefanrosetnosy: + rhettinger, ncoghlan, alexandre.vassalotti
2012-07-19 17:29:05mstefanrocreate