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: Docstring Style Local Override Does Not Work #183

Open
triley13 opened this issue Sep 4, 2024 · 5 comments
Open

bug: Docstring Style Local Override Does Not Work #183

triley13 opened this issue Sep 4, 2024 · 5 comments
Assignees
Labels
extractor: griffe Related to griffe refactor Change suggestion, not a bug nor a feature.

Comments

@triley13
Copy link

triley13 commented Sep 4, 2024

Description of the bug

Local override docstring_style for global configuration doesn't work.

To Reproduce

Declare in mkdocs.yaml:

plugins:
  - mkdocstrings:
      enabled: true
      default_handler: python
      handlers:
        python:
          options:
            docstring_style: google

-->

Inside file.md

::: path.to.module
    options:
      docstring_style: sphinx

Full traceback

Full traceback
N/A

Expected behavior

Override the default google style with the sphinx style for that one module.

Environment information

  • System: Windows-10-10.0.19045-SP0
  • Python: cpython 3.10.11
  • Environment variables:
  • Installed packages:
    • mkdocs v1.6.0
    • mkdocstrings v0.26.0
    • mkdocstrings-python v1.10.5
    • griffe v0.47.0

Additional context

I am using path: to load the module and I'm using mkdocs-monorepo-plugin.

@triley13 triley13 added the unconfirmed This bug was not reproduced yet label Sep 4, 2024
@pawamoy
Copy link
Member

pawamoy commented Sep 5, 2024

Thanks for the report @triley13 🙂

I suppose you expect all objects within path.to.module to be parsed as Sphinx-style, right? Currently we only re-assign the style to the specific object but not its members.

Ideally we would distinguish that a local option was used, and instead of re-assigning a style, we would use the provided local style to parse docstrings. The issue is that we're not able to differentiate a global option from a local one once we're in the render method of the handler 🤔

@pawamoy
Copy link
Member

pawamoy commented Sep 5, 2024

We can re-assign the style to the object and all its members, but note that Griffe extensions that modify the cached and parsed version of the docstring will probably stop working (because such extensions would be initially parsing and caching the docstring with the global style, not the local one).

WDYT?

@triley13
Copy link
Author

Hey @pawamoy, sorry I've been on vacation for a bit. Is caching necessary? If it's not, then I would suggest making an implementation that only works without caching. Otherwise, is there a workaround for parsing the docstrings for a single page, and limiting the style change to a particular webpage?

@pawamoy
Copy link
Member

pawamoy commented Nov 26, 2024

Hey again @triley13, thanks for your answer. I'm very inclined to do what you say and stop caching parsed versions of docstrings. Extensions should then never modify the parsed versions but only the "raw" docstring value. This is probably much less convenient though, programmatically speaking, since that means extensions have to manipulate a string instead of a proper data structure, but I'll think about it. Un-caching this is a no-brainer, it creates too many issues.

I'll mark this as a refactor.

@pawamoy pawamoy added extractor: griffe Related to griffe refactor Change suggestion, not a bug nor a feature. and removed unconfirmed This bug was not reproduced yet labels Nov 26, 2024
@pawamoy
Copy link
Member

pawamoy commented Dec 5, 2024

I have opened a relevant issue in Griffe, see mkdocstrings/griffe#340 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extractor: griffe Related to griffe refactor Change suggestion, not a bug nor a feature.
Projects
None yet
Development

No branches or pull requests

2 participants