[bug] mkdocstrings-python not publishing docstrings for function overloads

[mmacy]

Confirm this is an issue with the Python library and not an underlying OpenAI API

• This is an issue with the Python library

Describe the bug

Integrated mkdocstrings-insiders in #1 to get chat.completions.create() function signature overloads published, but the docstrings for the overloads aren't being published. To get docstrings to publish at all, I had to add docs to the overloaded function.

Not sure whether this is as-designed, expected, or a bug in mkdocstrings, but it warrants investigation. To note, the signatures for the overloads ARE published as expected (they weren't before moving to -insiders), but not their docstrings.

At the very least, I need to take a good look at chat.completions.create() and see how best to workaround it. Right now I'm likely not listing everything that needs listing in the attribute or arg lists for the overloaded create() function.

To Reproduce

1. Use computer
2. Profit

Code snippets

No response

OS

macOS

Python version

Python 3.11.7

Library version

openai v1.13.0

[pawamoy]

mkdocstrings maintainer here, chiming in :) I'm not a big user of overloads, so I initially decided to stack signatures and render only the doctstring of the implementation. I wasn't aware people wrote docstrings in the overloads. How would you like them to be rendered? Stacked signatures, like now, and concatenated docstrings? In source order or implementation first? Any feedback/suggestion is welcome!

[mmacy]

Wow - hi, @pawamoy! Great to hear from you. 🙂

Before I answer how I'd like it rendered, we should probably determine whether making a change to mkdocstrings is even the right thing to do. I'm not exactly a Python pro, so I'd first defer to someone with more experience than I whether doc'ing overloads is Pythonic, an anti-pattern, or somewhere in between. I'd hate to have you or someone else spend time hacking support into mkdocstrings for something someone shouldn't be doing in the first place.

The code that surfaced the issue is here in completions.Create() and its overloads:

src/openai/resources/chat/completions.py#L41-L696

They're using https://github.com/stainless-api to generate their SDKs, so presumably the docstrings-in-the-overloads behavior is coming from the Stainless SDK generator. I don't know whether that's a knob they can tweak in Stainless or if WYSIWG and that's that, but every overload is getting the full pile of docstrings and the implementation gets none.

My gut feeling as a dev is that I'd expect either all the docstrings to be in the implementation, or most would be in the implementation and then only specializations would appear in the overloads. But, as I say, I'd want someone who's been deep into Python for awhile to make the call on which (if either) is the most Pythonic.

[pawamoy]

Thank you for sharing this very useful context!

To me, the most Pythonic thing to do is to write everything in the implementation doctstring, and nothing in the overloads. That's because once you're in the REPL (Python interactive interpreter), or more generally at runtime, only the implementation is visible, since it shadows the overloads. Overloads are only visible to static analysis tools.

In the current situation, trying to print help for the create method would show nothing except the signature of the method:

>>> help(Completions.create)
create(...) -> ...

...whereas if the docstring was written in the implementation docstring, it'd be printed nicely. That's also why every parameter from every overload should be documented there: otherwise these docs would be lost at runtime.

So, I'd definitely recommend checking out if the stainless-api can instead add the docstring to the final implementation rather than the overloads. Bonus: it'll remove many duplicated lines :)

[mmacy]

Agreed on all points. Just did a diff on the docstrings that are in the overloads, and except for the position of stream in the arg list, they're identical.

So, I'd definitely recommend checking out if the stainless-api can instead add the docstring to the final implementation rather than the overloads. Bonus: it'll remove many duplicated lines :)

Stainless is a bit of a black box to me since I'm neither on their team nor OpenAI's, so I think the best we can do is check with @rattrayalex to see if this is an as-designed/won't fix thing, or if it should be submitted as an issue somewhere.

Either way, I'm kinda twice-removed from the library, so even if there's a knob to turn in Stainless that'd stuff the docstring into the implementation rather than the overloads, that would be a knob OpenAI would have to turn. Said knob is well outside my reach. 😛

[rattrayalex]

Hi there. You're welcome to open an issue on the openai-python repo to add a docstring to the implementation and we may do that.

However, I would urge you to consider displaying the overloaded docstring in preference of the implementation docstring when both exist. The reason is simple: the overloaded docstring documents the function as the user is calling it!

e.g.,

@overload
def foo(a: str, b: int):
  '''
  Args:
    a  : a string that does A stuff
    b  : an integer that does B stuff
  '''

@overload
def foo(c: str, d: int):
  '''
  Args:
    c  : a string that does C stuff
    d  : an integer that does D stuff
  '''

I would want to be ale to see proper docs for this.

[pawamoy]

@rattrayalex thanks for chiming in!

What do you think of the fact that such docstrings are lost at runtime, and therefore cannot be displayed to users in a REPL or in a Jupyter notebook?

Also, to continue with your example, since the implementation will declare all these parameters anyway (a, b, c, and d), why not documenting them all in the implementation?

I agree with your "as the user is calling it" point though. I see that VSCode does show the docstrings of the overloads when they exist. I'll think about it.

[pawamoy]

I also see that Ruff recommends against adding docstrings to overloads (though they do not really explain why):

Function decorated with @overload shouldn't contain a docstring Ruff(D418)

I opened astral-sh/ruff#10071

[pawamoy]

In the end maybe I'd recommend doing something like this:

from typing import overload


@overload
def f(a: int, b: str, x: bool) -> str:
    """Specific summary.

    Specific body.
    
    Args:
        a: Specific description.
        b: Specific description.
        x: Common description.
    """
    ...


@overload
def f(c: int, d: str, x: bool) -> str:
    """Specific summary.

    Specific body.
    
    Args:
        c: Specific description.
        d: Specific description.
        x: Common description.
    """
    ...


def f(
    x: bool,
    a: int | None = None,
    b: str | None = None,
    c: int | None = None,
    d: str | None = None,
) -> str:
    """Generic summary.

    Generic body.

    Args:
        a: Generic description.
        b: Generic description.
        c: Generic description.
        d: Generic description.
        x: Common description.
    """
    return b or d

I'd just have to find the best way to render that in docs 🙂

A few things to take into account for mkdocstrings:

• we can have only one entry in the object inventory for function f
• we can have only one entry in the object inventory for each parameter (a, b, c, d, x)
• but maybe we can suffix links and ids such that parameters from overload 1 link to descriptions in overload 1, etc.

I'll open an issue in our bugtracker, I invite you to continue the discussion there (don't want to spam @mmacy here 😅)

[rattrayalex]

Terrific, thank you! Appreciate the research and thoughtfulness here. I agree with your proposal about we should do and I'll file a ticket internally. It might be less trivial than you'd think but we would like to do it in our case.

Regardless, glad to hear that you'll look into supporting the overloads!

[mmacy]

Love the collaboration on this, @pawamoy and @rattrayalex.

Ruff recommends against adding docstrings to overloads (though they do not really explain why)

I would guess Ruff's D418 rule is as much about providing "defensive" guidance rather than it being a purely technical thing. With the way the @overload decorator behaves in Python, specifically this (emphasis mine):

@overload-decorated definitions are for the benefit of the type checker only, since they will be overwritten by the non-@overload-decorated definition.

It's probably a measure to ensure the function implementation ALWAYS has a docstring, since without that rule, we can end up in a situation like this where the implementation has none but the decorator-marked functions do, but they get overwritten by the implementation and thus - no docstring.

Also, it looks like we're not the only ones discussing how to handle @overload-decorated function docstrings:

sphinx-doc/sphinx#7787