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

Support non-multiple and non-named values in Yields and Receives #322

Merged
merged 3 commits into from
Sep 9, 2024
Merged

Support non-multiple and non-named values in Yields and Receives #322

merged 3 commits into from
Sep 9, 2024

Conversation

the-13th-letter
Copy link
Contributor

@the-13th-letter the-13th-letter commented Sep 8, 2024

This PR fixes #263 by applying the same logic from the Returns section parser to the Yields and Receives parser. The Yields section parser is configured with the same two configuration variables returns_multiple_items and returns_named_value, whereas the Receives section parser is configured via the new receives_multiple_items and receives_named_value configuration variables. Furthermore, the parsing of multiple or single docstring block contents, of named or unnamed parameters, and the unpacking of generator and/or tuple return annotations has been refactored into separate functions.


Contents:

  • 8a78e68 duplicates the non-multiple return and the named value parsing support for the Yields and Receives section parsers, essentially via copy-and-paste. Includes suitable tests, based on the Returns section tests.
  • 756118f implements the aforementioned refactored functions. This commit is functionally optional, but hopefully useful with respect to future ease of maintenance.
  • e01447a fixes linting errors.
  • 8e7cb30 adds documentation for the above features.

@the-13th-letter
Copy link
Contributor Author

the-13th-letter commented Sep 8, 2024

Concerning code style, I come from a black/ruff environment, and found no settings in your code repository. So my formatting and linting may be off.

EDIT: nevermind, your CI setup told me where to find the config settings, and what linting errors to fix.

@the-13th-letter the-13th-letter marked this pull request as draft September 8, 2024 10:33
@the-13th-letter the-13th-letter marked this pull request as ready for review September 8, 2024 11:05
Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! I appreciate the effort on splitting commits to ease review 🙂

src/_griffe/docstrings/google.py Outdated Show resolved Hide resolved
Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update.

Honestly I feel like the "aspect" check is overkill, I'd just prefer we remove it as well as the enum 🙂

@the-13th-letter
Copy link
Contributor Author

GitHub doesn't seem to recognize a PR that has been reset to an earlier, already-reviewed state as already reviewed…

Also, there's an apparently unrelated, failing CI test on Py3.13/Ubuntu-lowest-direct that's been haunting this PR since its inception.

@pawamoy
Copy link
Member

pawamoy commented Sep 9, 2024

Don't mind the 3.13-lowest failure, it's unrelated indeed and it's allowed to fail.

@pawamoy
Copy link
Member

pawamoy commented Sep 9, 2024

Concerning code style, I come from a black/ruff environment, and found no settings in your code repository. So my formatting and linting may be off.

Just saw this, sorry. There's a make vscode command you can use to configure VSCode for the project 🙂 It overrides local files. It's optional, you can run make format and make check anyway to format code and check it.

@pawamoy
Copy link
Member

pawamoy commented Sep 9, 2024

Erm sorry I pushed a dirty commit. Let me rebase locally and force-push then merge.

…s parsing

The Returns, Yields and Receives section parsers only really differ in
their fallbacks and the names of their configuration settings, not so
much in their general parsing behavior and the expected formatting of
the section contents. This commit is an attempt to factor out the
following functionality:

  * Read the section contents as a single block, or as multiple blocks,
    depending on the `multiple` setting.
  * Parse each block's first line as a named parameter, or an unnamed
    parameter, depending on the `named` setting.
  * Unpack `Generator` and `Iterator` types in the return annotation.
    Optionally error out if the return annotation is not of these types.
  * Optionally destructure the return tuple if `multiple` is in effect.

Issue-263: #263
@pawamoy pawamoy merged commit c9a78ce into mkdocstrings:main Sep 9, 2024
25 of 26 checks passed
@pawamoy
Copy link
Member

pawamoy commented Sep 9, 2024

Thanks again!

pawamoy added a commit that referenced this pull request Sep 12, 2024
…s annotation from parents

Follow-up-of-PR-322: #322
@pawamoy
Copy link
Member

pawamoy commented Sep 12, 2024

I refactored the logic again in a80bd3c, I think it's even more readable now.

@the-13th-letter
Copy link
Contributor Author

That's fine. I wasn't sure if I should touch the "where is the annotation looked up in?" part, so my version doesn't quite reap the same readability benefits as yours.

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.

bug: Google docstrings: no support for non-multiple or non-named values in Yields section
2 participants