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

bug: two issues with parsing Google-style Deprecated sections #352

Closed
the-13th-letter opened this issue Jan 16, 2025 · 1 comment · Fixed by #353
Closed

bug: two issues with parsing Google-style Deprecated sections #352

the-13th-letter opened this issue Jan 16, 2025 · 1 comment · Fixed by #353
Assignees
Labels
bug Something isn't working griffe: docstrings Related to docstring parsing

Comments

@the-13th-letter
Copy link
Contributor

the-13th-letter commented Jan 16, 2025

Description of the bug (1 of 2)

  1. Griffe claims to specifically support "Deprecated" sections in Google-style docstrings, but actually parses them as general admonitions. (See the "To Reproduce" section below.) The parsing code is there, but it isn't being triggered, because "deprecated" is not recognized as a section name: it is missing from the table of known section names.

    _section_kind = {
    "args": DocstringSectionKind.parameters,
    "arguments": DocstringSectionKind.parameters,
    "params": DocstringSectionKind.parameters,
    "parameters": DocstringSectionKind.parameters,
    "keyword args": DocstringSectionKind.other_parameters,
    "keyword arguments": DocstringSectionKind.other_parameters,
    "other args": DocstringSectionKind.other_parameters,
    "other arguments": DocstringSectionKind.other_parameters,
    "other params": DocstringSectionKind.other_parameters,
    "other parameters": DocstringSectionKind.other_parameters,
    "raises": DocstringSectionKind.raises,
    "exceptions": DocstringSectionKind.raises,
    "returns": DocstringSectionKind.returns,
    "yields": DocstringSectionKind.yields,
    "receives": DocstringSectionKind.receives,
    "examples": DocstringSectionKind.examples,
    "attributes": DocstringSectionKind.attributes,
    "functions": DocstringSectionKind.functions,
    "methods": DocstringSectionKind.functions,
    "classes": DocstringSectionKind.classes,
    "modules": DocstringSectionKind.modules,
    "warns": DocstringSectionKind.warns,
    "warnings": DocstringSectionKind.warns,
    }

    I'm preparing a PR to address this. PR fix: Parse Deprecated sections in Google-style docstrings #353 addresses this issue.

To Reproduce (1 of 2)

Using mkdocstrings-python 1.13.0 (with MkDocs 1.6.1 and mkdocs-material 9.5.49) and griffe 1.5.4, the following Python function

def griffe_bug() -> None:
    """bla bla bla

    Deprecated:
        Since v1.0: Insecure! Do not use!
    """

renders as

(an admonition)

Changing the section heading to DeprecatedABC changes the rendered admonition heading to "DeprecatedABC", confirming that this is indeed parsed as an admonition.

The tests in #353 confirm this suspicion as well.

Description of the bug (2 of 2)

If point (1) is fixed by adding the "deprecated" entry into the recognized section names table, then Griffe will assume the section contents are of the form version: text, and will attempt to partition the contents in this manner. If this fails, then Griffe will issue a warning, and then silently drop the entire section. This is because the parsing code, upon failing to partition the section text, returns None instead of a DocstringSection object (L711).

# check the presence of a name and description, separated by a semi-colon
try:
version, text = text.split(":", 1)
except ValueError:
docstring_warning(docstring, new_offset, f"Could not parse version, text at line {offset}")
return None, new_offset
version = version.lstrip()
description = text.lstrip()
return (
DocstringSectionDeprecated(version=version, text=description),
new_offset,
)

To Reproduce (2 of 2)

This is only confirmed via the tests in #353. No published version of Griffe exhibits this behavior yet.

Expected behavior

Griffe should parse these sections as Deprecated sections, not as admonitions. mkdocstrings-python should render them not as default admonitions, but as specific ones (perhaps similar to how griffe-deprecations renders them).

Griffe should not drop docstring sections it cannot properly parse, it should always pass them on.

Environment information

Debian Linux "testing", Python 3.12. See reproduction #1 above for Griffe and mkdocs* versions.

Additional context

(none)

@the-13th-letter the-13th-letter added the unconfirmed This bug was not reproduced yet label Jan 16, 2025
@pawamoy pawamoy added bug Something isn't working griffe: docstrings Related to docstring parsing and removed unconfirmed This bug was not reproduced yet labels Jan 16, 2025
@pawamoy
Copy link
Member

pawamoy commented Jan 16, 2025

Awesome report @the-13th-letter, thank you very much 🙂 I agree with everything. I'll review your PR now!

pawamoy pushed a commit that referenced this issue Jan 17, 2025
The code to parse these sections already existed, but the "Deprecated" section header was missing from the table of known section headers. Being an unknown section, Griffe then treated this as an admonition.

Griffe initially required a version indicator in Google-style Deprecated sections. This change now allows it to be missing, in which case the version is the empty string.

Issue-352: #352
PR-353: #353
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working griffe: docstrings Related to docstring parsing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants