jbms / sphinx-immaterial

Adaptation of the popular mkdocs-material material design theme to the sphinx documentation system
https://jbms.github.io/sphinx-immaterial/
Other
200 stars 32 forks source link

Ignore exceptions in autodoc_property_type #142

Closed jbms closed 2 years ago

jbms commented 2 years ago

Fixes #134.

jbms commented 2 years ago

I haven't tested this but I think it will fix the issue.

2bndy5 commented 2 years ago

sadly, this does fix the issue. It just hides the errros.

.. autoclass:: pyrf24.rf24_network.RF24NetworkHeader
    :members: to_node type from_node id reserved to_string

    .. property:: next_id
        :classmethod:

        The next sequential identifying number used for the next created header's `id`.

renders as image


FYI: Only to_string is a function in that RF24NetworkHeader class. All the rest of the members are attributes. Custom signatures only applies to functions in pybind11, so I'm not entirely convinced it has something to do with me using custom signatures in the embedded docstrings.

2bndy5 commented 2 years ago

We can probably get rid of the unused variable:

--- a/sphinx_immaterial/apidoc/python/autodoc_property_type.py
+++ b/sphinx_immaterial/apidoc/python/autodoc_property_type.py
@@ -26,7 +26,6 @@ def _get_property_getter(obj: Any) -> Any:

 def _get_property_return_type(obj: Any) -> Optional[str]:
-    fget = _get_property_getter(obj)
     doc = obj.__doc__
     if doc is None:
         return None

I've got a working theory (assuming fget is a builtin function) for a fix here:

--- a/sphinx_immaterial/apidoc/python/autodoc_property_type.py
+++ b/sphinx_immaterial/apidoc/python/autodoc_property_type.py
@@ -19,7 +19,7 @@ property_sig_re = re.compile("^(\\(.*)\\)\\s*->\\s*(.*)$")
 def _get_property_getter(obj: Any) -> Any:
     # property
     func = sphinx.util.inspect.safe_getattr(obj, "fget", None)
-    if func is None:
+    if func is None or getattr(func, "__text_signature__", None) is None:
         # cached_property
         func = sphinx.util.inspect.safe_getattr(obj, "func", None)
     return func
2bndy5 commented 2 years ago

When verifying the CI in #145, I added some config for black to the toml. This config alters the exit code returned by black. Previously, it was allowing unformatted code to pass in CI, but now it is properly alerting us to unformatted code.

Unfortunately, I don't see the file tests/python_apigen_test_modules/_alpha.py that it is complaining about here

2bndy5 commented 2 years ago

I found the file on the main branch. Its weird that black scans the files that only exist on base branch.

2bndy5 commented 2 years ago

This seems to be because the CI runs on both PR sync events and any push event. The PR sync events are the ones that black scans the base branch (resulting in alerts about files that don't exist on the head branch).

jbms commented 2 years ago

Thanks --- I rebased this branch, and fixed the formatting issue.

jbms commented 2 years ago

I also fixed the bug with the return types not being listed for pybind11 properties, and added some unit tests to prevent regressions in the future.

2bndy5 commented 2 years ago

I'm good to go here. I'm thinking of pushing a tag v0.9.0 (after this is merged) due to the added graphviz integration. Unless you prefer v0.8.2?

jbms commented 2 years ago

Pushing a tag sounds good.