davebshow / goblin

A Python 3.5 rewrite of the TinkerPop 3 OGM Goblin
Other
93 stars 21 forks source link

Interaction with Sphinx and Napoleon: `__qualname__` #51

Closed rosenbrockc closed 7 years ago

rosenbrockc commented 7 years ago

I am writing documentation for a project using Sphinx. Here is the first issue causing the build to fail. I'll post the second in another issue:

  File "/lib/python3.5/site-packages/sphinx/ext/autodoc.py", line 862, in filter_members
    not keep, self.options)
  File "/lib/python3.5/site-packages/sphinx/application.py", line 593, in emit_firstresult
    for result in self.emit(event, *args):
  File "/lib/python3.5/site-packages/sphinx/application.py", line 589, in emit
    results.append(callback(self, *args))
  File "/lib/python3.5/site-packages/sphinxcontrib/napoleon/__init__.py", line 428, in _skip_member
    qualname = getattr(obj, '__qualname__', '')
  File "/lib/python3.5/site-packages/goblin/mapper.py", line 213, in __getattr__
    value, self._element_type))
goblin.exception.MappingError: unrecognized property __qualname__ for class: edge

In mapper.py, line 206, I hacked it together this way (so that my build will pass and I can keep working; the generated documentation still looks good):

    def __getattr__(self, value):
        try:
            mapping, _ = self._ogm_properties[value]
            return mapping
        except:
            if value == "__qualname__":
                return self.__class__.__qualname__

            raise exception.MappingError(
                "unrecognized property {} for class: {}".format(
                    value, self._element_type))

It seems strange to me that a class instance in python 3 would not have __qualname__.

davebshow commented 7 years ago

An instance of a class does not have an attribute __qualname__, while a class does:

In [1]: class A:
   ...:     pass
   ...: 

In [2]: getattr(A, '__qualname__')
Out[2]: 'A'

In [3]: a = A()

In [4]: getattr(a, '__qualname__')
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-3-b3b9d698fe71> in <module>()
----> 1 getattr(a, '__qualname__')

The fact that the Goblin class attribute __mapping__ is an instance of Mapping seems to throw a wrench in napoleon's member checking for doc inclusion (though I haven't looked into this in depth):

In [5]: from goblin import Edge

In [6]: class MyEdge(Edge):
   ...:     pass
   ...: 

In [7]: getattr(MyEdge.__mapping__, '__qualname__')
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
/home/davebshow/git/goblin/goblin/mapper.py in __getattr__(self, value)
    207         try:
--> 208             mapping, _ = self._ogm_properties[value]
    209             return mapping

KeyError: '__qualname__'

During handling of the above exception, another exception occurred:

MappingError                              Traceback (most recent call last)
<ipython-input-8-0082f3e0d57c> in <module>()
----> 1 getattr(MyEdge.__mapping__, '__qualname__')

/home/davebshow/git/goblin/goblin/mapper.py in __getattr__(self, value)
    211             raise exception.MappingError(
    212                 "unrecognized property {} for class: {}".format(
--> 213                     value, self._element_type))
    214 
    215     def _map_properties(self, properties):

MappingError: unrecognized property __qualname__ for class: edge

Based on Python's data model, I believe this is the expected behavior in this case. Maybe this could be fixed in the napoleon source code by checking isinstance(obj, type) before looking for __qualname__...but again, I haven't looked into it really, so I'm not positive

rosenbrockc commented 7 years ago

I agree with your analysis. I just looked into the napoleon source code and their documentation indicates that obj is expected to be "module, class, exception, function, method, or attribute". They clearly weren't thinking of the case where a class attribute is actually a class instance. Adding isinstance(obj, type) should fix that. Though it may be a little more complicated since functions are technically not an instance of type. I'll submit an issue on napoleon for this.

davebshow commented 7 years ago

Hi @rosenbrockc. I was just curious, what project is this for? I am interested to know how you are using goblin. Thanks!