Use isdatadescriptor instead of isgetsetdescriptor

[jorisvandenbossche]

This is a small change I did to our vendored version in pandas. Opening directly an PR instead of an issue as it is easier to show with the code. The reason for this is is because we have some properties in pandas defined on a DataFrame et al that are properties defined in cython. For those isinstance(param_obj, property) does not work (as expected), and inspect.isgetsetdescriptor(param_obj)) also not, but apparently inspect.isdatadescriptor(param_obj)) does recognize them.

So although this is a useful change for pandas, I am not really sure about the possible implications. I don't really know the difference between "datadescriptor" and "getsetdescriptor", but from the docs it seems datadescriptor is a bit more general and also contains getsetdescriptor: https://docs.python.org/3.6/library/inspect.html#inspect.isdatadescriptor

(the safer option would actually be to add the check, instead of replacing)

[jnothman]

Seems a reasonable change. Can it be tested easily without c extensions

[jorisvandenbossche]

Yes, it does not need to be from cython. Quick test that illustrates it:

In [22]: class Property(object):
    ...: 
    ...:     def __init__(self, axis=0, doc=""):
    ...:         self.axis = axis
    ...:         self.__doc__ = doc
    ...: 
    ...:     def __get__(self, obj, type):
    ...: 
    ...:         if obj is None:
    ...:             # Only instances have _data, not classes
    ...:             return self
    ...:         else:
    ...:             axes = obj._data.axes
    ...:         return axes[self.axis]
    ...: 
    ...:     def __set__(self, obj, value):
    ...:         obj._set_axis(self.axis, value)
    ...:         

In [23]: class A():
    ...:     
    ...:     attr = Property(doc="test attribute")
    ...: 

In [24]: A.attr
Out[24]: <__main__.Property at 0x7f97100de0f0>

In [25]: isinstance(A.attr, property)
Out[25]: False

In [26]: import inspect

In [27]: inspect.isgetsetdescriptor(A.attr)
Out[27]: False

In [28]: inspect.isdatadescriptor(A.attr)
Out[28]: True

In [29]: A.attr.__doc__
Out[29]: 'test attribute'

[jorisvandenbossche]

Is this the best way to test it?

On branch:

In [3]: from numpydoc.docscrape_sphinx import get_doc_object

In [5]: print(get_doc_object(A))

Test docstring




:Attributes:

    attr
        test attribute

(and then test the appropriate substring is in there)

while on master:

In [3]: from numpydoc.docscrape_sphinx import get_doc_object

In [4]: print(get_doc_object(A))

Test docstring

[jorisvandenbossche]

I added a test case.
The line above is from our original example in pandas, and actually does not matter (it's the if obj is None: return self that matters, so I can also remove this

[rgommers]

LGTM, merging. Thanks @jorisvandenbossche