search

readthedocs / sphinx-autoapi

A new approach to API documentation in Sphinx.
https://sphinx-autoapi.readthedocs.io/
MIT License
432 stars 128 forks source link

fix: Unknown type: placeholder bug #180 #394

Closed rickstaa closed 2 months ago

rickstaa commented 1 year ago

This PR adds a fix for the Unknown type: placeholder that was given in #180. See this comment.

AWhetter commented 1 year ago

Do you know what situation this bug is triggered in? Would you be able to add a test for it?

rickstaa commented 1 year ago

@AWhetter, I can add code in this bug example package to the tests folder and create a test based on that 👍🏻.

However, please remember that this use case is not typical in regular Python packages but does come up when people use external builders that combine Python modules programmatically (e.g. catkin). I tried to see whether this bug comes up using a native namespace package. Still, it only is triggered when people deviate from the recommendations and add a __init__.py at the top folder of the namespace subpackage (see https://github.com/rickstaa/unknown_type_placeholder_bug_ns) 🤔. I yet have to find a more standard python use case where this problem happens.

rickstaa commented 1 year ago

@AWhetter Why this happens is quite easy to see by looking at this bug example package. The current codebase only expects to encounter a module once. As a result, the code in

https://github.com/readthedocs/sphinx-autoapi/blob/0ac1e113594908bff61c0a95baa6b1899c5ecdc3/autoapi/mappers/python/mapper.py#L333

overwrites all the entries that were mapped in the first pass. This use case is rarely encountered when working with standard Python packages but is encountered frequently when working with ROS packages. I'm happy to add the code found in this bug example package to the tests folder if you think it is generic enough.

AWhetter commented 2 months ago

Closing due to lack of follow up. Please feel free to resubmit if you are able to add a test case.