mkdocstrings / python

A Python handler for mkdocstrings.
https://mkdocstrings.github.io/python
ISC License
185 stars 33 forks source link

bug: Typing.Overload from pyi file not showing in generated docs #182

Open nvoxland-al opened 2 months ago

nvoxland-al commented 2 months ago

Description of the bug

I have methods defined with @typing.overload but they are not showing up in the generated docs. Other methods show up fine, but not the overload ones. There are no warnings. I am using the insiders version of mkdocstring-python and griffe

My docs are generated from definitions like this in my pyi file:

@typing.overload
def add_column(
    self, name: str, type: types.DataType, format: formats.DataFormat
) -> None:
    """
    Add a new column to the data container
    """

@typing.overload
def add_column(self, name: str, type: types.HumanType) -> None:
    """
    Add a new column to the data container
    """

@typing.overload
def add_column(self, name: str, type: types.DataType) -> None:
    """
    Add a new column to the data container
    """

NOTE: There is not any backing .py file because this is a wrapper around a c++ python module. So the mkdocstrings generation is purely going against the .pyi files.

To Reproduce

.pyi file with the definitions above

Expected behavior

Expect the overloaded methods to show in the docs

Environment information

Additional context

pawamoy commented 2 months ago

Hi @nvoxland-al, thanks for the report!

Without the implementation, Griffe indeed doesn't know where to attach the overloads (and they are simply never used by mkdocstrings-python).

I see two solutions:

I would love to get your feedback on the second solution :relaxed: If you would like to try solution 1 instead, I can try to provide more guidance, and will try to answer any question you have of course :slightly_smiling_face:

pawamoy commented 2 months ago

Just rendering the overloads as regular functions is not possible because they share the same name/path, and we can't have members with the same name, by design, either in Griffe or in the object inventories generated by mkdocstrings. Happy to hear about any other solution you can think of!

nvoxland-al commented 2 months ago

Thanks for the followup, @pawamoy .

You are saying that the way it overall works is that while regular methods can be picked out from the .pyi files, for overloaded methods it needs to look at the actual available method implementations? And those actual methods can either be in python files, or via an installed native module? Because the docs show correctly for all the regular methods documented in the .pyi files when those are all that mkdocstrings can see.

For option 1: I was hoping to avoid having to build the c++ module in order to manage the docs. We have people we'd like to help with the docs who won't have the whole c++ build environment available. So I'd like to avoid that.

For option 2: If we did have the overloaded methods in py files instead of .pyi the docs would have shown correctly? If you are generating fake implementations based on what you see in the pyi files are you able to use the signatures from there as if we had specified the different overload methods in the py files?

pawamoy commented 2 months ago

The data provenance (.pyi or .py files) actually doesn't matter: we load from both and merge data together.

The issue here is that only the overloads are available, and not the implementation (for legitimate reasons, as you explained). And overloads have to be attached to a Function object in order for mkdocstrings to render them. This Function object is supposed to represent the implementation, which we don't have here.

In short, with only the overload signatures, Griffe has incomplete data that mkdocstrings doesn't know how to render.


Since you'd like to avoid building the C++ module, let's go with a fake implementation and an empty signature, which will be the least incorrect or confusing thing to show to readers.

I'll send you some code tomorrow for a Griffe extension that you'll be able to use locally (without having to publish it on PyPI) :)

pawamoy commented 2 months ago

You could try something like this:

# griffe_exts.py
from typing import Any

import griffe

class FixOrphanOverloads(griffe.Extension):
    """Dispatch remaining overloads to their respective functions."""

    def _dispatch_overloads(self, mod_or_cls: griffe.Module | griffe.Class) -> None:
        for name, overloads in mod_or_cls.overloads.items():
            if overloads:
                try:
                    function: griffe.Function = mod_or_cls.members[name]  # type: ignore[assignment]
                except KeyError:
                    function = griffe.Function(name)
                    mod_or_cls.set_member(name, function)
                function.overloads = overloads
                del mod_or_cls.overloads[name]

    def on_module_members(self, *, mod: griffe.Module, **kwargs: Any) -> None:  # noqa: ARG002
        """Dispatch remaining overloads to their respective functions."""
        self._dispatch_overloads(mod)

    def on_class_members(self, *, cls: griffe.Class, **kwargs: Any) -> None:  # noqa: ARG002
        """Dispatch remaining overloads to their respective functions."""
        self._dispatch_overloads(cls)

Put that code in a file somewhere and load it in mkdocs.yml:

plugins:
- mkdocstrings:
    handlers:
      python:
        options:
          extensions:
          - path/to/griffe_exts.py

With this, your overloads should start showing up.

Note however that overload support is still pretty basic, you can subscribe to this issue: https://github.com/mkdocstrings/python/issues/135.

nvoxland-al commented 1 month ago

Thanks @pawamo, sorry it took me a while to get back to you. Back on the documentation focus now...

When I add your extension, it fails with this error:

ERROR   -  Error reading page 'api/dataset.md': dictionary changed size during iteration
Traceback (most recent call last):
  File "/usr/local/bin/mkdocs", line 8, in <module>
    sys.exit(cli())
             ^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mkdocs/__main__.py", line 272, in serve_command
    serve.serve(**kwargs)
  File "/usr/local/lib/python3.11/site-packages/mkdocs/commands/serve.py", line 85, in serve
    builder(config)
  File "/usr/local/lib/python3.11/site-packages/mkdocs/commands/serve.py", line 67, in builder
    build(config, serve_url=None if is_clean else serve_url, dirty=is_dirty)
  File "/usr/local/lib/python3.11/site-packages/mkdocs/commands/build.py", line 310, in build
    _populate_page(file.page, config, files, dirty)
  File "/usr/local/lib/python3.11/site-packages/mkdocs/commands/build.py", line 167, in _populate_page
    page.render(config, files)
  File "/usr/local/lib/python3.11/site-packages/mkdocs/structure/pages.py", line 285, in render
    self.content = md.convert(self.markdown)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/markdown/core.py", line 357, in convert
    root = self.parser.parseDocument(self.lines).getroot()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/markdown/blockparser.py", line 117, in parseDocument
    self.parseChunk(self.root, '\n'.join(lines))
  File "/usr/local/lib/python3.11/site-packages/markdown/blockparser.py", line 136, in parseChunk
    self.parseBlocks(parent, text.split('\n\n'))
  File "/usr/local/lib/python3.11/site-packages/markdown/blockparser.py", line 158, in parseBlocks
    if processor.run(parent, blocks) is not False:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mkdocstrings/extension.py", line 128, in run
    html, handler, data = self._process_block(identifier, block, heading_level)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mkdocstrings/extension.py", line 216, in _process_block
    data: CollectorItem = handler.collect(identifier, options)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mkdocstrings_handlers/python/handler.py", line 322, in collect
    loader.load(
  File "/usr/local/lib/python3.11/site-packages/_griffe/loader.py", line 179, in load
    top_module = self._load_package(package, submodules=submodules)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/_griffe/loader.py", line 511, in _load_package
    top_module = self._load_module(package.name, package.path, submodules=submodules)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/_griffe/loader.py", line 535, in _load_module
    return self._load_module_path(module_name, module_path, submodules=submodules, parent=parent)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/_griffe/loader.py", line 565, in _load_module_path
    self._load_submodules(module)
  File "/usr/local/lib/python3.11/site-packages/_griffe/loader.py", line 570, in _load_submodules
    self._load_submodule(module, subparts, subpath)
  File "/usr/local/lib/python3.11/site-packages/_griffe/loader.py", line 603, in _load_submodule
    submodule = self._load_module(
                ^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/_griffe/loader.py", line 535, in _load_module
    return self._load_module_path(module_name, module_path, submodules=submodules, parent=parent)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/_griffe/loader.py", line 559, in _load_module_path
    module = self._visit_module(module_name, module_path, parent)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/_griffe/loader.py", line 636, in _visit_module
    module = visit(
             ^^^^^^
  File "/usr/local/lib/python3.11/site-packages/_griffe/agents/visitor.py", line 112, in visit
    ).get_module()
      ^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/_griffe/agents/visitor.py", line 204, in get_module
    self.visit(top_node)
  File "/usr/local/lib/python3.11/site-packages/_griffe/agents/visitor.py", line 213, in visit
    getattr(self, f"visit_{ast_kind(node)}", self.generic_visit)(node)
  File "/usr/local/lib/python3.11/site-packages/_griffe/agents/visitor.py", line 244, in visit_module
    self.extensions.call("on_module_members", node=node, mod=module, agent=self)
  File "/usr/local/lib/python3.11/site-packages/_griffe/extensions/base.py", line 313, in call
    getattr(extension, event)(**kwargs)
  File "/docs/plugins/griffe_extensions.py", line 22, in on_module_members
    self._dispatch_overloads(mod)
  File "/docs/plugins/griffe_extensions.py", line 10, in _dispatch_overloads
    for name, overloads in mod_or_cls.overloads.items():
RuntimeError: dictionary changed size during iteration

Where my copy of your extension is at /docs/plugins/griffe_extensions.py

To control the order and structure of the md file, I'm specifying the classes to render, sort of like this:


## Create and Load Dataset
::: deeplake.create
::: deeplake.open
::: deeplake.open_read_only

## Dataset API

::: deeplake.Dataset

::: deeplake.ReadOnlyDataset

::: deeplake.DatasetView

::: deeplake.ColumnView

::: deeplake.Row

::: deeplake.RowRange

::: deeplake.RowView

::: deeplake.RowRangeView

Any idea of what is causing the error?

pawamoy commented 1 month ago

dictionary changed size during iteration

Right, just need to iterate on a copy of the dict to prevent this error :)

for ... in dict(....items()):
    ...
nvoxland-al commented 1 month ago

Thanks. Putting it as for ... in dict(....).items(): gets it to run correctly and I think gets me to the #135 issue where I want the docs to include the docstrings/parameters-table/examples from each of my overridden pyi functions rather than just listing the signatures.

With my setup using the griffle overload to generate the fake implementation, do I have an ability to custom blend the content from each of the overloaded versions into the single generated function?

It seems to work in theory with the docstring but not parameters?

If I modify my function to:

class FixOrphanOverloads(griffe.Extension):
    """Dispatch remaining overloads to their respective functions."""

    def _dispatch_overloads(self, mod_or_cls: griffe.Module | griffe.Class) -> None:
        for name, overloads in dict(mod_or_cls.overloads).items():
            if overloads:
                try:
                    function: griffe.Function = mod_or_cls.members[name]  # type: ignore[assignment]
                except KeyError:
                    function = griffe.Function(name)
                    function.docstring = griffe.Docstring("Generated Docstring")
                    function.parameters.add(griffe.Parameter("test1"))
                    function.parameters.add(griffe.Parameter("test2"))
                    mod_or_cls.set_member(name, function)

                function.overloads = overloads
                del mod_or_cls.overloads[name]  

my generated docs has "Generated Docstring" after my signatures (I'm using separate_signature: True) but the parameters don't show.

Is there a way to attach generated parameters and examples the generated function like that?

nvoxland-al commented 1 month ago

I maybe spoke a bit too soon. I think I see what's going on...

Even in your example, the docs now generate an additional version of the function with no arguments (because we are generating function = griffe.Function(name) which has no arguments, right?

And in my example with the function.parameters.add(griffe.Parameter("test1")) then the added version of the function has a "test1" argument instead of no arguments. Maybe if I create that parameter with more settings like it's own docstring, it will show up in a parameters table?

BUT: is there a way to make that stub implementation object not show up in the docs? Is del mod_or_cls.overloads[name] supposed to be doing that, but not working? Or is that doing something different?

I did try just using the function from the 1st overload as the actual function:

    def _dispatch_overloads(self, mod_or_cls: griffe.Module | griffe.Class) -> None:
        for name, overloads in dict(mod_or_cls.overloads).items():
            if overloads:
                try:
                    function: griffe.Function = mod_or_cls.members[name]  # type: ignore[assignment]
                except KeyError:
                    function = overloads[0]
                    mod_or_cls.set_member(name, function)
                function.overloads = overloads
                del mod_or_cls.overloads[name]

but then I get a duplicate version of one of the signatures.

pawamoy commented 1 month ago

do I have an ability to custom blend the content from each of the overloaded versions into the single generated function?

It should be possible yes, but as noted earlier: "Griffe is not able to combine multiple signatures into one (it's typing territory and probably far from trivial)". You are of course free to try anyway to combine parameters from all overloads into the fake implementation :slightly_smiling_face:

If you add parameters to the function manually, you still have to reference them in the docstring for them to be rendered. You can either use a specific docstring style (google, numpydoc, sphinx) and set the function docstring's parser and parser_options accordingly (function.docstring.parser = "google"), or you could mutate the parsed version of the docstring (which is a cached property):

function.docstring.parsed.append(griffe.DocstringSectionParameters(...))

See https://mkdocstrings.github.io/griffe/reference/api/docstrings/models/#griffe.DocstringSectionParameters.


BUT: is there a way to make that stub implementation object not show up in the docs?

I wonder: instead of creating a fake implementation, then hiding it from the docs, couldn't you simply declare your real implementation signature in the .pyi file, with an ellipsis as body?

@overload
def function(...) -> ...: ...

@overload
def function(...) -> ...: ...

# signature of the implementation
def function(...) -> ...: ...

I believe this would not overwrite the actual implementation for users, and this would provide correct information to both Griffe and type-checkers.

Though maybe you'd still want to hide the implementation signature from the docs?

nvoxland-al commented 1 month ago

Yes, I'd still want to hide the implementation signature from the docs. I just want the users to see the overloaded variations.

Is there a way to mark the implementation function signature as hidden from the docs? Or to even programmatically exclude certain signatures from being rendered in the docs via an extension?

pawamoy commented 1 month ago

Yes, I'd still want to hide the implementation signature from the docs. I just want the users to see the overloaded variations.

I see, thanks. Sounds like a valid request. I'll think about implementing an option for it.

Is there a way to mark the implementation function signature as hidden from the docs? Or to even programmatically exclude certain signatures from being rendered in the docs via an extension?

You could override templates and make use of Griffe objects' extra attribute for storing metadata, then use this metadata in your overridden templates.

nvoxland-al commented 1 month ago

I wonder: instead of creating a fake implementation, then hiding it from the docs, couldn't you simply declare your real implementation signature in the .pyi file, with an ellipsis as body?

Yes, that does generate the docs correctly, and probably easier to manage for us than programmatically generating the stub master version.

You could override templates and make use of Griffe objects' extra attribute for storing metadata, then use this metadata in your overridden templates.

Thanks, I'll take a look at that