New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AttributeError when Cython functions are loaded #25

Closed
jamadden opened this Issue Aug 16, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@jamadden
Member

jamadden commented Aug 16, 2018

Even on Python 2 certain Cython objects have a __qualname__ attribute, but the value of that attribute is not a string, it's a 'getset_descriptor', at least when accessed on the class. This breaks apidoc badly with 500 ISE's for all the interfaces:

  File "//src/zope/app/apidoc/ifacemodule/browser.py", line 176, in getGenericRequiredAdapters
    for reg in regs]
  File "//src/zope/app/apidoc/component.py", line 194, in getAdapterInfoDictionary
    if isReferencable(path):
  File "//src/zope/app/apidoc/utilities.py", line 186, in isReferencable
    if hasattr(obj, '__class__') and getPythonPath(obj.__class__) == path:
  File "//src/zope/app/apidoc/utilities.py", line 138, in getPythonPath
    qname = qname.split('.')[0]
AttributeError: 'getset_descriptor' object has no attribute 'split'

Here, we're looking the path passed to isReferencable points to a function compiled by cython (<cyfunction default_externalized_object_factory_finder_factory at 0x10c04f710>) (I believe it's a registered utility or adapter). It has a class of <type 'cython_function_or_method'>, which has a __qualname__ attribute, which, when accessed, returns <attribute '__qualname__' of 'cython_function_or_method' objects>, a "getset_descriptor", not a string.

    if hasattr(naked, '__qualname__'):
        # Python 3. This makes unbound functions inside classes
        # do the same thing as they do an Python 2: return just their
        # class name.
        qname = naked.__qualname__
        qname = qname.split('.')[0] # <--- BAM!

This makes the entire Interface tree of the apidoc a series of broken links.

@jamadden jamadden added the bug label Aug 16, 2018

@mgedmin

This comment has been minimized.

Show comment
Hide comment
@mgedmin

mgedmin Aug 17, 2018

Member

That sounds like a Cython bug to me? I'm not opposed to working around it (e.g. change the hasattr check to isinstance(getattr(naked, '__qualname__', None), str)) but perhaps we should report it to Cython developers?

Member

mgedmin commented Aug 17, 2018

That sounds like a Cython bug to me? I'm not opposed to working around it (e.g. change the hasattr check to isinstance(getattr(naked, '__qualname__', None), str)) but perhaps we should report it to Cython developers?

@jamadden

This comment has been minimized.

Show comment
Hide comment
@jamadden

jamadden Aug 17, 2018

Member

I agree the behaviour is weird at best on Cython's part. I can and will file an issue there too, I just wanted to understand a bit better what was happening first.

It turns out it may not be an easy fix. __qualname__ is established with a PyGetSetDef for tp_getset, which takes pointers to functions that operate on the instance but that don't even get invoked when looked up on the class. CPython automatically wraps the PyGetSetDef structs with a getset_descriptor that wraps those functions up and takes care of returning itself when looked up on the class, as good descriptors do (TIL). So this would have to throw that away and implement a custom descriptor as an object that gets put into the class's __dict__.

Moreover it will take a while to propagate. The PyGetSetDef code is copied into every module compiled with Cython, so everyone would have to upgrade their version of Cython and make releases.

Member

jamadden commented Aug 17, 2018

I agree the behaviour is weird at best on Cython's part. I can and will file an issue there too, I just wanted to understand a bit better what was happening first.

It turns out it may not be an easy fix. __qualname__ is established with a PyGetSetDef for tp_getset, which takes pointers to functions that operate on the instance but that don't even get invoked when looked up on the class. CPython automatically wraps the PyGetSetDef structs with a getset_descriptor that wraps those functions up and takes care of returning itself when looked up on the class, as good descriptors do (TIL). So this would have to throw that away and implement a custom descriptor as an object that gets put into the class's __dict__.

Moreover it will take a while to propagate. The PyGetSetDef code is copied into every module compiled with Cython, so everyone would have to upgrade their version of Cython and make releases.

@jamadden

This comment has been minimized.

Show comment
Hide comment
@jamadden

jamadden Aug 17, 2018

Member

Ohho, this seems to only be an issue on Python 2. On Python 3, the regular rules for __qualname__ seem to trump the custom PyGetSetDef. Python 2 functions don't even have __qualname__ so maybe the Cython dev's could be convinced to #ifdef that out on Python 2.

Member

jamadden commented Aug 17, 2018

Ohho, this seems to only be an issue on Python 2. On Python 3, the regular rules for __qualname__ seem to trump the custom PyGetSetDef. Python 2 functions don't even have __qualname__ so maybe the Cython dev's could be convinced to #ifdef that out on Python 2.

jamadden added a commit that referenced this issue Aug 17, 2018

@jamadden jamadden closed this in #26 Aug 17, 2018

jkmartindale added a commit to jkmartindale/zope.app.apidoc that referenced this issue Aug 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment