laurent-laporte-pro / deprecated

Python @deprecated decorator to deprecate old python classes, functions or methods.
MIT License
307 stars 34 forks source link

isinstance() fails with versionadded and versionchanged #48

Closed jdogburck closed 3 years ago

jdogburck commented 3 years ago

Expected Behavior

Using isinstance() for classes annotated with the deprecated library should work as expected, regardless of the number of annotations.

from deprecated.sphinx import versionadded
from deprecated.sphinx import versionchanged
from deprecated.sphinx import deprecated

@versionadded(version='X.Y', reason='some reason')
class VersionAdded:
    pass

@versionadded(version='X.Y', reason='some reason')
class VersionAddedChild(VersionAdded):
    pass

@versionchanged(version='X.Y', reason='some reason')
class VersionChanged:
    pass

@versionchanged(version='X.Y', reason='some reason')
class VersionChangedChild(VersionChanged):
    pass

@deprecated(version='X.Y', reason='some reason')
class Deprecated_:
    pass

@deprecated(version='Y.Z', reason='some reason')
class DeprecatedChild_(Deprecated_):
    pass

@versionadded(version='X.Y')
@versionchanged(version='X.Y.Z')
class AddedChanged:
    pass

# should all be True
print([isinstance(VersionAddedChild(), VersionAddedChild), isinstance(VersionAddedChild(), VersionAdded)])
print([isinstance(VersionChangedChild(), VersionChangedChild), isinstance(VersionChangedChild(), VersionChanged)])
print([isinstance(DeprecatedChild_(), DeprecatedChild_), isinstance(DeprecatedChild_(), Deprecated_)])
print([isinstance(AddedChanged(), AddedChanged)])

Actual Behavior

Tell us what happens instead.

[False, False]
[False, False]
[True, True]   # seems to have been addressed on  0e944e0
[False]

Environment

tantale commented 3 years ago

Typically, for the versionadded and versionchanged directives, the decorator returns the class unchanged (except, of course, the docstring). So it must be transparent to the insinstance function.

As for the deprecated decorator, the call to __new__ is additionally modified to issue a warning. But, again, isinstance should work. Weird…

Have you done your own diagnosis of the problem, which might help?

jdogburck commented 3 years ago

i haven't done' any diagnosis beyond the diagnostic code attached. here's another small snippet that adds some insight into why isinstance() is failing... (based on the original code)

print([(type(x()), hex(id(type(x()))), x) for x in [Deprecated_, VersionAdded, VersionChanged]])

yields

[(<class '__main__.Deprecated_'>, '0x25fbedad588', <class '__main__.Deprecated_'>), 
(<class '__main__.VersionAdded'>, '0x25fbedabf68', <AdapterWrapper at 0x0000025FC0582208 for type at 0x0000025FBEDABF68>), 
(<class '__main__.VersionChanged'>, '0x25fbedace28', <AdapterWrapper at 0x0000025FC0520438 for type at 0x0000025FBEDACE28>)]

👍 The type of \@deprecated class instances is the annotated class.

👎 The type of \@versionadded and \@versionchanged class instances is a wrapper to the annotated class.