[Python-Dev] Callable, non-descriptor class attributes.

[Thomas Wouters]

One of the things brought up at the language summit (and I believe at the VM
summit, although I wasn't there) was the unpredictable behaviour of
callables turning into methods when they're class attributes. Specifically,
things that are CFunctions in CPython (builtin functions, which are not
descriptors and do not turn into methods) but could be plain Python
functions in other Python implementations. (Or even turn into a plain
function in CPython itself, if the other-implementation argument isn't
persuasive enough for you.) To see the kind of thing that would be
(potentially) problematic, see for example Lib/encodings/ascii.py:

import codecs

### Codec APIs

class Codec(codecs.Codec):

    # Note: Binding these as C functions will result in the class not
    # converting them to methods. This is intended.
    encode = codecs.ascii_encode
    decode = codecs.ascii_decode

(I hope the irony of a two-line comment explaining the situation, as opposed
to the self-documenting nature of staticmethod, isn't lost on anyone :)
Obviously, if the codecs.ascii_encode and codecs.ascii_decode functions are
replaced *for whatever reason* by Python functions, this will break. Because
these kinds of class attributes are often used for dependency injection, the
potentially problematic cases are even more common in tests -- the io
testsuite, for example, specialcases pyio.open (in two different ways, no
less) but omits the same for io.open for no apparent reason (other than that
right now, in CPython, it isn't needed.)

At the summit we discussed some potential improvements here:

 1. Make staticmethod a callable object directly (it isn't, currently) and
apply it to any Python function that replaces a (in CPython) CFunction. The
change to staticmethod may be a good idea regardless, but the policy of
making other implementations comply to this quirk in CPython seems (to me)
like unnecessary descrimination[*].
 2. Make CFunctions turn into methods in CPython (after a period of warning
about the impending change, obviously.) The actual *usecase* for this is
hard to envision (I think Guido thought someone might do 'class str(str):
len = len', which doesn't seem like that big an improvement :) so it would
be a feature for symmetry's sake only.
 3. Produce a CompatibilityWarning making sure that the programmer knows
that this particular thing might not do the same thing in other Python
implementations *or versions*.
 4. Make it an error to have a callable class attribute that isn't a
descriptor (although maybe we only discussed this one in my head.)

To gauge the size of the problem I've implemented solution #3: I've uploaded
a patch that adds the warning (currently as a DeprecationWarning) and fixes
a bunch of the issues (not yet all of them):
http://bugs.python.org/issue11470. The patch currently warns for callable,
non-descriptor, *non-class* class attributes. The class check is necessary
because injected classes and -- more popularly, in some ORMs -- nested
classes would otherwise produce the warning. (Considering replacing a class
with a function has lots of other effects, like isinstance checks failing, I
don't think warning for that case is necessary anyway.) An alternate
strategy would be to warn only about uses of CFunctions in these cases, but
this misses out on a whole slew of potential issues (e.g. a module global
that is currently a bound instance method.)

If you look at the patch you'll notice it also fixes a couple of "dubious"
cases -- usually in a module's own testsuite -- where one could certainly
argue that the test is allowed to make the assumptions it's making. I don't
think writing it explicitly future-proof detracts from the readability,
stability and maintainability in most of these cases, although I should
point out that multiprocessing's testsuite produces a bunch more warnings
and isn't as easy to fix because of its use of locals().update() in a class
statement. (Yes, really.)

With regards to changing staticmethod, in an earlier iteration of the patch
I modified staticmethod to be directly callable *and* to return self in its
tp_descr_get, and this had a side-effect I hadn't considered: code that used
a staticmethod-wrapped object in any way other than calling it, broke. Right
now staticmethod is implemented as a generic container to inhibit descriptor
behaviour, in spite of its name, so it can be used to protect any kind of
descriptor in a class attribute. I'm not sure how common that use is, and
whether we should try to provide a different wrapper for those cases.
Unfortunately, without changing staticmethod to return itself, rather than
the callable it's wrapping, in tp_descr_get, code like this:

  class C:
    open = builtin.open

  class D:
    open = C.open

... would still produce different behaviour when builtin.open changes from a
CPython CFunction to a Python function wrapped in a staticmethod: C.open
would produce the Python function, and D.open would then become a method
after all. (This is why _pyio has OpenWrapper instead, I guess.)

Considering all this, I personally think we should go for solution #3 or #4
(with a long migration path through #3, of course): add a warning like the
one I implemented, add plenty of documentation explaining the issue (perhaps
even pointing to a PEP in the warning,) and fix everything in the stdlib
that produces the warning, even if it's hard to see how that particular case
could become a problem. The warning should probably be of the same kind as
DeprecationWarning to prevent end-users from being confused by it. Changing
staticmethod to be directly callable may still be a good idea, but changing
what its descr_get does is not. Making CFunctions turn into bound methods is
something that seems of little value, but could be considered at a later
point (after the warning has had time to flush out most of the problematic
cases.)

[*]-sorry-couldn't-resist'ly y'rs,
-- 
Thomas Wouters <thomas at python.org>

Hi! I'm a .signature virus! copy me into your .signature file to help me
spread!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20110311/b7a3d382/attachment.html>