Skip to content
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

Dynamic Docstrings in Sync Client #1077

Closed
wants to merge 19 commits into from

Conversation

asonnenschein
Copy link
Contributor

This MR enables reference documentation for new synchronous client objects implemented in #1049, and uses a decorator to dynamically populate sync client docstrings from their async client counterparts.

@stephenhillier
Copy link
Contributor

Is there a way to get these to display in IDEs (I can't get the decorated ones to display in VSCode or Neovim)?

@asonnenschein
Copy link
Contributor Author

asonnenschein commented Nov 25, 2024

Is there a way to get these to display in IDEs (I can't get the decorated ones to display in VSCode or Neovim)?

Not that I am aware of. The way I have the docstrings set up in this PR will display them via Python's help() method, and they will build with the docs (not sure why I cannot reply to your comment in a thread @stephenhillier, which is why I am making a separate response comment).

@ischneider
Copy link
Member

I messed around with approaches for this and my findings are that the language servers (used for display in IDE) do not execute code (which makes sense!) so the doc annotation or my metaclass approach don't work.

What does work, and what I'm now advocating for is:

  • rather than wrap a client, inherit directly from it
  • for each async function, (re-)declare a sync version w/ the appropriate typing hint
  • use the super class function (async) and use the call_sync to wait for the coroutine
  • add a AsyncGenerator -> Generator converter to further reduce boiler plate
  • migrate tests to use an approach where we build a payload and execute in both a sync and async context to reduce boiler plate and ensure all functions are wrapped

Will iterate on this for the remainder of the day and see what I can do for one of the clients as an example...

@ischneider
Copy link
Member

While the approach I took "works" in code-completion and generated docs, it fails mypy type checking since the sync client overrides the async one and provides different type hints on return values (AsyncIterator vs Iterator)...

IMHO we should stick w/ the full docstring version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants