bslatkin / effectivepython

Effective Python: Second Edition — Source Code and Errata for the Book
https://effectivepython.com
2.2k stars 710 forks source link

Item 51: decorator isn't applied to `dict.__setitem__` method #105

Open hong-dev opened 2 years ago

hong-dev commented 2 years ago

I'm not sure if I understand the purpose correctly, but it seems that the code below skips the __setitem__ method when wrapping the decorator :hushed:

It seems that the type of dict.__setitem__ is wrapper_descriptor which is not in the trace_types, and the type of dict.__getitem__ is method_descriptor which is types.MethodDescriptorType in the trace_types.

So, when the code runs, the decorator isn't applied to dict.__setitem__ method :hushed: (The yellow highlighter in the code below)


Screenshot from 2021-11-01 14-43-59

dev-m1-macbook commented 2 years ago

(not the author here...)

yes you're right for python 3.5+ ( couldn't install python <= 3.4 -- sorry 😢 ) :

>>> type(dict.__getitem__)
<class 'method_descriptor'>
>>> type(dict.__setitem__)
<class 'wrapper_descriptor'>
>>> isinstance(dict.__setitem__, types.WrapperDescriptorType)
True

you can add types.WrapperDescriptorType to trace_types

trace_types = (
    types.MethodType,
    types.FunctionType,
    types.BuiltinFunctionType,
    types.BuiltinMethodType,
    types.MethodDescriptorType,
    types.ClassMethodDescriptorType,
    types.WrapperDescriptorType,  # <-- this
 )

tldr-explanation from stackoverflow: '~DescriptorType' : stuff from c-code (in cpython source code)

bslatkin commented 1 month ago

Thank you for the report! I agree this is missing __setitem__ and that's a problem. The implementation should be:

import types

TRACE_TYPES = (
    types.MethodType,
    types.FunctionType,
    types.BuiltinFunctionType,
    types.BuiltinMethodType,
    types.MethodDescriptorType,
    types.ClassMethodDescriptorType,
    types.WrapperDescriptorType,
)

IGNORE_METHODS = (
    '__repr__',
    '__str__',
)

class TraceMeta(type):
    def __new__(meta, name, bases, class_dict):
        klass = super().__new__(meta, name, bases, class_dict)

        for key in dir(klass):
            if key in IGNORE_METHODS:
                continue

            value = getattr(klass, key)
            if not isinstance(value, TRACE_TYPES):
                continue

            wrapped = trace_func(value)
            setattr(klass, key, wrapped)

        return klass