laike9m / pdir2

Pretty dir() printing with joy:beer:
MIT License
1.33k stars 47 forks source link

Logic of self._getattr() in PrettyDir not explicit #47

Closed liwt31 closed 5 years ago

liwt31 commented 5 years ago

The logic of current code:

https://github.com/laike9m/pdir2/blob/a8b416330728e00b5c5d2ea15182708962e64b67/pdir/api.py#L148-L157 There is a try-except structure in the implementation as a patch to solve cases when get_dict_attr(self.obj, name) fail. Before it is considered that get_dict_attr(self.obj, name) fails with some packages (#1). Actually it fails in a certain case when the obj parameter for pdir is a class. See the following example:

class A:
    base = 1
class B(A): pass
pdir(B)
# exception caught and `attrs[name] = getattr(self.obj, name)` is called

The problem arises from inheritance and __mro__. If a class is passed as an obj to pdir, what happens in attrs[name] = get_dict_attr(self.obj, name) is (code below):

  1. Tries to look up base in B.__dict__ and failed
  2. Tries to look up base in the __dict__ attribute of the first object in B.__class__.__mro__
    • B.__class__ is IOW type(B), which returns type.
    • There's no way base appear in the __dict__ of the first element in type's __mro__, which is also a type.
  3. Other objects in B.__class__.__mro__ won't work neither.
  4. Exception raises https://github.com/laike9m/pdir2/blob/a8b416330728e00b5c5d2ea15182708962e64b67/pdir/_internal_utils.py#L7-L11

    A fix to the issue

    def get_dict_attr(attr_obj, attr_name):
    if inspect.isclass(attr_obj):
        obj_list = [attr_obj] + list(attr_obj.__mro__)
    else:
        obj_list = [attr_obj] + list(attr_obj.__class__.__mro__)
    for obj in obj_list:
        if hasattr(obj, '__dict__') and attr_name in obj.__dict__:
            return obj.__dict__[attr_name]
    raise AttributeError

    If attr_obj is a class, the code will use __mro__ directly from the attr_obj instead of the __class__ of the attr_obj. The try...catch in self._getattr() won't be necessary with this fix.

    Comparision between the two implementation

    Their overall behavior are basically same, however, the try...catch and getattr patch can lead to weird behavior in some rare cases. For the following class definition:

    
    class A():
    @classmethod
    @property
    def a(cls):
        return 'a'

class B(A): pass

`pdir(A)` produces:

... descriptor: a: class classmethod with getter, classmethod(function) -> method

`pdir(B)` produces:

... function: a:

And the improved version of `get_dict_attr` uniformly produces:

... descriptor: a: class classmethod with getter, classmethod(function) -> method

laike9m commented 5 years ago

Looks good to me. Thanks for identifying the bug.