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

Allows Docstring to return a list of DocstringReturns to better work with numpydoc #43

Merged
merged 3 commits into from
Jul 21, 2021

Conversation

mayitbeegh
Copy link
Contributor

PR for #33
Numpydoc supports multiple return values in the return section. Here's an example:

Returns
-------
err_code : int
    Non-zero value indicates error code, or zero on success.
err_msg : str or None
    Human readable error message, or None on success.

This PR attempts to add support for multiple return values, since the library only supports a single return value at the moment.
The ideal solution would be updating the existing Docstring.returns to return T.List[DocstringReturns] instead of T.Optional[DocstringReturns]. This is a breaking change, but it's more consistent with how params work. The alternative solution is adding a new property that returns T.List[DocstringReturns], which is what's in this draft PR. I also added a few lines of unit test to showcase the use case. This is only a draft PR. Depending on how the maintainers feel about the backwards compatibility, things may go very differently.

@rr-
Copy link
Owner

rr- commented Jul 21, 2021

Props for not breaking backwards compatibility, plus I feel like having a separate returns for a single property is a good option since most of the codebases I'd worked with do not document alternative return values this way.

LMK if you're OK with merging and releasing.

@mayitbeegh
Copy link
Contributor Author

Thanks @rr-, I'd like to add more unit test code to keep up with the rest of the project.

@mayitbeegh mayitbeegh marked this pull request as ready for review July 21, 2021 21:23
@mayitbeegh
Copy link
Contributor Author

Done.

@rr- rr- merged commit 39fd120 into rr-:master Jul 21, 2021
@rr-
Copy link
Owner

rr- commented Jul 21, 2021

Thank you; released as 0.9

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.

None yet

2 participants