Author Darren.Dale
Recipients Darren.Dale, benjamin.peterson, daniel.urban, dsdale24, eric.araujo, ncoghlan, stutzbach
Date 2011-06-10.21:24:44
SpamBayes Score 5.55112e-17
Marked as misclassified No
Message-id <1307741086.78.0.526220927236.issue11610@psf.upfronthosting.co.za>
In-reply-to
Content
I posted the following at python-dev (http://mail.python.org/pipermail/python-dev/2011-June/111837.html):

I would like to try to address some shortfalls with the way python deals with
abstract base classes containing descriptors. I originally was just concerned
with improving support for defining abstract properties with the decorator
syntax and converting between abstract and concrete properties, but recently
realized that the problem extends to descriptors in general.

ABCs
----

First, a bit of background may be in order. An abstract base class is defined
by specifying its metaclass as ABCMeta (or a subclass thereof)::

   class MyABC(metaclass=ABCMeta):
       @abstractmethod
       def foo(self):
           pass

When trying to instantiate MyABC or any of its subclasses, ABCMeta inspects the
current class namespace for items tagged with __isabstractmethod__=True::

   class ABCMeta(type):
   #[...]
       def __new__(mcls, name, bases, namespace):
           cls = super().__new__(mcls, name, bases, namespace)
           # Compute set of abstract method names
           abstracts = {name
                        for name, value in namespace.items()
                        if getattr(value, "__isabstractmethod__", False)}

ABCMeta then checks if any of the base classes define any items tagged with
__isabstractmethod__ and whether they remain abstract in the current
class namespace::

           for base in bases:
               for name in getattr(base, "__abstractmethods__", set()):
                   value = getattr(cls, name, None)
                   if getattr(value, "__isabstractmethod__", False):
                       abstracts.add(name)
           cls.__abstractmethods__ = frozenset(abstracts)

In Objects/typeobject.c, __abstractmethods__ is actually a descriptor, and
setting it gives the type a chance to set an internal flag specifying if it
has any abstract methods defined. When object_new is called in typeobject.c,
the flag is checked and an error is raised if any abstract methods were
identified.

Issues with ABCs and descriptors
--------------------------------

In order for this scheme to work, ABCMeta needs to identify all of the abstract
methods, but there are some limitations when we consider descriptors. For
example, Python's property is a composite object, whose behavior is defined by
the getter, setter, and deleter methods with which it is composed. Since there
is already an @abstractmethod decorator, I would have suspected that defining
abstract properties would be intuitive::

   class MyABC(metaclass=ABCMeta):
       @abstractmethod
       def _get_foo(self):
           pass
       @abstractmethod
       def _set_foo(self, val):
           pass
       foo = property(_get_foo, _set_foo)

       @property
       @abstractmethod
       def bar(self):
           pass
       @bar.setter
       @abstractmethod
       def bar(self, val):
           pass

Ideally, one would want the flexibility of defining a concrete getter and an
abstract setter, for example. However, ABCMeta does not inspect the descriptors
of a class to see if they contain any abstract methods. It only inspects the
descriptor itself for a True __isabstractmethod__ attribute. This places the
burdon on every descriptor implementation to provide its own support for ABC
compatibility. For example, support for abstract properties was attempted by
adding abstractproperty to the abc module. abstractproperty subclasses the
property builtin (as opposed to the relationship between every other abstract
and concrete class in the python language). Here is the definition of
abstractproperty, in its entirety (modulo docstrings)::

   class abstractproperty(property):
       __isabstractmethod__ = True

A number of problems manifest with this approach, and I think they all can be
traced to the fact that the abstractedness of a descriptor is currently not
dependent upon the abstractedness of the methods with which it is
composed. The documentation for abstractproperty doesn't suggest using
@abstractmethod::

       class C(metaclass=ABCMeta):
           def getx(self): ...
           def setx(self, value): ...
           x = abstractproperty(getx, setx)

which leads to Issue #1: What is abstract about C.x? How does a subclass of C
know whether it needs to override the getter or setter?

Issue #2: The decorator syntax cannot be used to convert an abstract property
into a concrete one. (This relates to Issue #1: how would a descriptor even know
when such a conversion would be appropriate?) Running the following code::

   from abc import ABCMeta, abstractmethod, abstractproperty

   class AbstractFoo(metaclass=ABCMeta):
       @abstractproperty
       def bar(self):
           return 1
       @bar.setter
       def bar(self, val):
           pass

   class ConcreteFoo(AbstractFoo):
       @AbstractFoo.bar.getter
       def bar(self):
           return 1
       @bar.setter
       def bar(self, val):
           pass
   foo = ConcreteFoo()

yields::

   TypeError: Can't instantiate abstract class ConcreteFoo with abstract
   methods bar

Issue #3: The following class *is* instantiable, even though
AbstractFoo declared that a setter for bar is required::

   class ConcreteFoo(AbstractFoo):
       @property
       def bar(self):
           pass

Previous attempt to improve abc.abstractproperty
------------------------------------------------

It seems to me that the strategy used by abc.abstractproperty is fundamentally
ill-advised. I explored the possibility of extending abstractproperty,
redefining its getter, setter, and deleter methods such that they would work in
conjunction with the @abstractmethod decorator and yield an instance of the
builtin property once all abstract methods were replaced with concrete ones
(http://bugs.python.org/issue11610). Issues #1 and #2 were addressed, but there
were still problems with that approach. It did not address Issue #3, and it
also introduced a new issue, #4::

   class AbstractFoo(metaclass=ABCMeta):
       @abstractproperty
       # bar would be an abstractproperty, even though the getter is concrete
       def bar(self):
           return 1
       @bar.setter
       # bar.setter inspected the getter and the new setter, did not identify
       # any abstract methods, and thus returned an instance of the built-in
       # property
       def bar(self, val):
           pass
       @bar.deleter
       # bar is a concrete property, its deleter decorator does not know it
       # is supposed to check for abstract methods, so it will return an
       # instance of the built-in property:
       @abstractmethod
       def bar(self):
           pass

By the time the deleter was specified, bar was a concrete property, which does
not know it should return an instance of abstractproperty (in part because the
inheritance diagram for property/abstractproperty is inverted). Thus,
AbstractFoo was instantiable, even though it shouldn't be.

Finally, issue #5: the current approach taken by ABCMeta and abstractproperty
places the burdon on descriptors to identify themselves to ABCMeta as
abstract. Considering the issues encountered with abstractproperty, this may be
an onerous requirement.

There has been a fair amount of discussion at http://bugs.python.org/issue11610
, which can be summarized as a) concerns about maintaining backward
compatibility, and b) objections to requiring @abstractmethod to specify that a
method being passed to abstractproperty is abstract.

Extending ABCMeta: A Promising Way Forward
------------------------------------------

I think the key is to focus on Issue #3. ABCMeta needs to be improved to
recognize descriptor objects, both in the current namespace as well as the base
classes, and to identify any abstract methods associated with the
descriptors. I suggest the following approach in ABCMeta::

   def __new__(mcls, name, bases, namespace):
       cls = super().__new__(mcls, name, bases, namespace)
       # Compute set of abstract method names

       def isdescriptor(val):
           return hasattr(val, '__get__') or hasattr(val, '__set__') \
                   or hasattr(val, '__delete__')
       def getabstracts(ns):
           return [name
                   for name, value in ns.items()
                   if getattr(value, "__isabstractmethod__", False)]

       abstracts = getabstracts(namespace)
       for item, val in namespace.items():
           # further inspect descriptors for abstract methods:
           if isdescriptor(val):
               ## unfortunately, can't import inspect:
               #from inspect import getmembers
               #d = dict(getmembers(val))
               ## so instead, use the following:
               d = dict((k, getattr(val, k, None)) for k in dir(val))
               for name in getabstracts(d):
                   # add the abstract descriptor methods to the list:
                   abstracts.append('%s.%s'%(item, name))
       for base in bases:
           for name in getattr(base, "__abstractmethods__", set()):
               if '.' in name:
                   # base class identified a descriptor abstract method:
                   k, v = name.split('.')
                   desc = getattr(cls, k, None)
                   val = getattr(desc, v, None)
               else:
                   val = getattr(cls, name, None)
               if val is None or getattr(val, "__isabstractmethod__", False):
                   abstracts.append(name)
       cls.__abstractmethods__ = frozenset(abstracts)

I rolled everything into __new__ just to keep it as simple as possible for the
sake of discussion. Python already provides the rest of the framework needed
for descriptors to work properly with ABCs. This implementation actually works;
I've tested it with an existing python-3.2 install::

   from abc import ABCMeta, abstractmethod

   class AbstractFoo(metaclass=ABCMeta):
       @property
       @abstractmethod
       def bar(self):
           return 1
       @bar.setter
       @abstractmethod
       def bar(self, val):
           pass

   >>> abstractfoo = AbstractFoo()
   Traceback (most recent call last):
     File "temp.py", line 17, in <module>
       abstractfoo = AbstractFoo()
   TypeError: Can't instantiate abstract class AbstractFoo with abstract
   methods bar.fget, bar.fset

as expected. Note the more informative error message indicating what about the
bar property is abstract. Also::

   class ConcreteFoo(AbstractFoo):
       @AbstractFoo.bar.getter
       def bar(self):
           return 1

   >>> foo = ConcreteFoo()
   Traceback (most recent call last):
     File "temp.py", line 24, in <module>
       foo = ConcreteFoo()
   TypeError: Can't instantiate abstract class ConcreteFoo with abstract
   methods bar.fset

So issue #1 is addressed, since we are explicitly specifying which descriptor
methods are abstract. Issue #2 has been addressed, since the following class is
instantiable::

   class ConcreteFoo(AbstractFoo):
       @AbstractFoo.bar.getter
       def bar(self):
           return 1
       @bar.setter
       def bar(self, val):
           pass

Issue #3 is also addressed. In the following example, even though I redefine
the bar property as a readonly property, ABCMeta recognizes that a setter is
needed::

   class ConcreteFoo(AbstractFoo):
       @property
       def bar(self):
           return 1

   >>> foo = ConcreteFoo()
   Traceback (most recent call last):
     File "temp.py", line 24, in <module>
       foo = ConcreteFoo()
   TypeError: Can't instantiate abstract class ConcreteFoo with abstract
   methods bar.fset

Issue #4 (introduced in a previous attempt to solve the problem
using abstractproperty) is also addressed::

   class AbstractFoo(metaclass=ABCMeta):
       @property
       def bar(self):
           return 1
       @bar.setter
       def bar(self, val):
           pass
       @bar.deleter
       @abstractmethod
       def bar(self):
           pass

   >>> abstractfoo = AbstractFoo()
   Traceback (most recent call last):
     File "temp.py", line 15, in <module>
       abstractfoo = AbstractFoo()
   TypeError: Can't instantiate abstract class AbstractFoo with abstract
   methods bar.fdel

Finally, this approach addresses Issue #5 by holding ABCMeta responsible for
identifying the abstractedness of descriptor methods, rather than placing that
burdon on the desciptor objects to identify themselves as abstract. If ABCMeta
were extended as above to identify abstract methods associated with
descriptors, third parties would simply decorate methods used to compose the
descriptors with @abstractmethod.

This change to ABCMeta would not effect the behavior of abstractproperty, so
backward compatibility would be maintained in that respect. But I think
abstractproperty should be deprecated, or at the very least removed from the
documentation. The documentation for @abstractmethod in >=python-3.3 should be
extended to provide examples with properties/descriptors. The syntax would be
backward compatible with older python versions, but with <python-3.3 ABCMeta
would simply not recognize descriptors' abstract methods. This leads to one
source of potential forward compatibility::

   class AbstractFoo(metaclass=ABCMeta):
       @property
       @abstractmethod
       def bar(self):
           return 1

   class ConcreteFoo(AbstractFoo):
       pass

Both above classes would be instantiable with <python-3.3, but not with
>=python3.3. In my opinion, this is a feature: python-3.3 has identified a bug
in ConcreteFoo. The developer would not have tagged that method as abstract
unless they had intended for consumers of AbstractFoo to provide a concrete
implementation.
History
Date User Action Args
2011-06-10 21:24:47Darren.Dalesetrecipients: + Darren.Dale, ncoghlan, benjamin.peterson, stutzbach, eric.araujo, daniel.urban, dsdale24
2011-06-10 21:24:46Darren.Dalesetmessageid: <1307741086.78.0.526220927236.issue11610@psf.upfronthosting.co.za>
2011-06-10 21:24:46Darren.Dalelinkissue11610 messages
2011-06-10 21:24:44Darren.Dalecreate