GrahamDumpleton / wrapt

A Python module for decorators, wrappers and monkey patching.
BSD 2-Clause "Simplified" License
2.06k stars 231 forks source link

Cannot use issubclass on decorated class derived from abc.ABC #130

Open SeppMe opened 5 years ago

SeppMe commented 5 years ago

Not sure if this is an issue with wrapt or abc. The following code will raise a TypeError: issubclass() arg 1 must be a class instead of printing True as I would expect:

import wrapt
import abc

class Base(abc.ABC):
    pass

@wrapt.decorator
def pass_through(wrapped, instance, args, kwargs):
    return wrapped(*args, **kwargs)

@pass_through
class Klass(Base):
    pass

print(issubclass(Klass, Base))

Apparently, abc.ABC replaces the subclasscheck with its own special subclasscheck, which in turn raises the exception. Unfortunately, that subclasscheck is some internal function that I cannot debug. The last arguments for this internal subclasscheck that the debugger can see are <class '__main__.Base'> and <class '__main__.Klass'>, which looks fine to me.

The example above can be simplified even further to

import wrapt
import abc

@wrapt.decorator
def pass_through(wrapped, instance, args, kwargs):
    return wrapped(*args, **kwargs)

@pass_through
class Klass(abc.ABC):
    pass

print(issubclass(Klass, abc.ABC))

but I chose the first example because it demonstrates better what I want to achieve.

GrahamDumpleton commented 5 years ago

Adding a decorator to a class presents a few issues which there are no solutions for. One is deriving from a decorated class. Another would be taking the type of the decorated class and using equality on it to see if something else matches the same rather than using issubclass(). Best practice would be that issubclass() should be used. Code using exact type equality checks is a bad idea as it breaks duck typing. If the Python abstract base classes are doing something that is breaking duck typing that isn't helpful.

Anyway, it falls into the same class of problem as:

I try and dig into it further when I can. It sounds like the check is implemented in a C code extension if you can't track it through with a Python debugger.

MelnykAndriy commented 5 years ago
import abc

import wrapt

class Base(metaclass=abc.ABCMeta):

    @classmethod
    @abc.abstractmethod
    def method(cls):
        pass

class B(Base):
    @classmethod
    def method(cls):
        print(f'In {cls.__name__}')

class C(B):
    @classmethod
    def method(cls):
        print(f'In {cls.__name__}')

if __name__ == '__main__':
    print(f'Ok {issubclass(C, B)}')
    print(f'Not ok {issubclass(wrapt.ObjectProxy(C), B)}')

The code above raises TypeError: issubclass() arg 1 must be a class after updating from 3.6.7 to 3.7.3.

GrahamDumpleton commented 3 years ago

Fiddling with this a bit and using an example that contrasts with when abc.ABCMeta isn't used get:

import abc
import wrapt

@wrapt.decorator
def wrapper(wrapped, instance, args, kwargs):
    return wrapped(args, kwargs)

class B1:
    @classmethod
    def method(cls):
        print(f'In {cls.__name__}')

@wrapper
class C1(B1):
    @classmethod
    def method(cls):
        print(f'In {cls.__name__}')

class D1(C1):
    @classmethod
    def method(cls):
        print(f'In {cls.__name__}')

print(f'B1.__subclasses__() = {B1.__subclasses__()}')

print(f'issubclass(C1, B1) {issubclass(C1, B1)}')
print(f'C1.__subclasses__() {C1.__subclasses__()}')

print(f'issubclass(D1, B1) {issubclass(D1, B1)}')
print(f'D1.__subclasses__() {D1.__subclasses__()}')

class Base(metaclass=abc.ABCMeta):

    @classmethod
    @abc.abstractmethod
    def method(cls):
        pass

class B2(Base):
    @classmethod
    def method(cls):
        print(f'In {cls.__name__}')

@wrapper
class C2(B2):
    @classmethod
    def method(cls):
        print(f'In {cls.__name__}')

class D2(C2):
    @classmethod
    def method(cls):
        print(f'In {cls.__name__}')

print(f'B2.__subclasses__() = {B2.__subclasses__()}')

print(f'issubclass(C2, B2) {issubclass(C2, B2)}')
print(f'C2.__subclasses__() {C2.__subclasses__()}')

print(f'issubclass(D2, B2) {issubclass(D2, B2)}')
print(f'D2.__subclasses__() {D2.__subclasses__()}')

with output of:

B1.__subclasses__() = [<class '__main__.C1'>]
issubclass(C1, B1) True
C1.__subclasses__() [<class '__main__.D1'>]
issubclass(D1, B1) True
D1.__subclasses__() []
B2.__subclasses__() = [<class '__main__.C2'>]
Traceback (most recent call last):
  File "/Users/graham/Projects/wrapt-packages/wrapt-tests/subclass.py", line 57, in <module>
    print(f'issubclass(C2, B2) {issubclass(C2, B2)}')
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/abc.py", line 102, in __subclasscheck__
    return _abc_subclasscheck(cls, subclass)
TypeError: issubclass() arg 1 must be a class

This is because the very first thing that abc override of issubclass() does is:

static PyObject *
_abc__abc_subclasscheck_impl(PyObject *module, PyObject *self,
                             PyObject *subclass)
/*[clinic end generated code: output=b56c9e4a530e3894 input=1d947243409d10b8]*/
{
    if (!PyType_Check(subclass)) {
        PyErr_SetString(PyExc_TypeError, "issubclass() arg 1 must be a class");
        return NULL;
    }

If it got past that strict type, it may well still work like the case where abc.ABCMeta isn't involved.

GrahamDumpleton commented 3 years ago

From looking at Python implementation found enough justification to believe should report a bug against Python standard library. They have a pure Python implementation which actually does the right thing, but it is only used for pypy. Bug report is:

I have still made some changes to wrapt to handle some other cases with issubclass() not specifically related to ABCMeta implementation. This will make it into 1.13.0. You can see tests from wrapt show what does and doesn't work at: