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: mkdocstrings filters option doesn't work anymore after upgrade to griffe 0.46.0 #294

Closed
kkpattern opened this issue Jun 17, 2024 · 18 comments
Assignees
Labels
unconfirmed This bug was not reproduced yet

Comments

@kkpattern
Copy link

Description of the bug

I don't know it's a griffe bug or the mkdocstrings's bug. After upgrade to griffe 0.46.0, the filters option doesn't work anymore. We have the filters option set to a empty list. After upgrade to griffe 0.46.0, the protected methods are no longer collected. Revert back to 0.45.3 fixed the issue.

To Reproduce

With the following setting:

plugins:
  - mkdocstrings:
      handlers:
        python:
          options:
            docstring_style: sphinx
            show_source: false
            filters: []

Protected methods are no longer collected.

Expected behavior

Projected methods can be collected.

Environment information

griffe --debug-info  # | xclip -selection clipboard
- __System__: Linux-4.19.0-10-amd64-x86_64-with-glibc2.31
- __Python__: cpython 3.11.3 (/tmp/.venv/bin/python3)
- __Environment variables__:
- __Installed packages__:
  - `griffe` v0.46.0

Additional context

@kkpattern kkpattern added the unconfirmed This bug was not reproduced yet label Jun 17, 2024
@pawamoy
Copy link
Member

pawamoy commented Jun 17, 2024

Thanks for the fast report. Looking into it right now.

@pawamoy
Copy link
Member

pawamoy commented Jun 17, 2024

OK, found the issue.

In the Python handler templates, we first filter objects according to the members, inherited_members, filters and show_if_no_docstring options. Then, we do a second filtering pass, checking if each object is either explicitly passed in members, or is public. This is because when you don't pass an explicit members list, and let filters do their stuff, you still don't want to end up with non-public objects rendered in your page (such as imported objects).

The issue is that previously, we would tweak the is_public check so as to ignore the object's name, but now it always checks the name, so objects whose name starts with _ are not considered public and are therefore filtered out. That means dunder attributes and methods are filtered out.

I think the easiest and most correct way to fix this is to update Griffe's "public" logic to consider dunder attributes and methods to be public, because even if we don't access them directly, we still use them publicly through different syntax or means.

I'll do just that!

pawamoy added a commit that referenced this issue Jun 17, 2024
….) to be public

The reasoning is that is a special object, even though it is not accessed directly, provides public functionality: comparing objects, multiplying them, printing their Python representation or stringified version, etc..

Issue-294: #294
Issue-295: #295
@pawamoy
Copy link
Member

pawamoy commented Jun 17, 2024

OK, v0.46.1 is on PyPI, let me know if you still encounter issues!

@pawamoy pawamoy closed this as completed Jun 17, 2024
@llucax
Copy link

llucax commented Jun 17, 2024

WORKSFORME ❤️

@vulli321
Copy link

does not work for me, 0.46.1 only fixed #295
protected methods as noted in this issue are still missing

@pawamoy pawamoy reopened this Jun 17, 2024
@pawamoy
Copy link
Member

pawamoy commented Jun 17, 2024

Ah, right, I read it as "special" instead of protected, my bad. Essentially I think the Python handler should stop doing this second filtering pass. But that would be a breaking change, so I'll simply change it to filter out imported objects, not private/protected ones.

@pawamoy
Copy link
Member

pawamoy commented Jun 18, 2024

@vulli321 could you try latest mkdocstrings-python (1.10.4) and tell me if this issue is fixed?

@kkpattern
Copy link
Author

Upgrade to mkdocstrings-python (1.10.4) fixed our issue. Thanks!

@vulli321
Copy link

vulli321 commented Jun 18, 2024

can confirm, fixed now 👍

@pawamoy
Copy link
Member

pawamoy commented Jun 18, 2024

Thanks @kkpattern, @vulli321! Closing :)

@llucax
Copy link

llucax commented Jun 19, 2024

Whoops, it seems like this issue doesn't want to go away. The new release broke my use case again 😬

@llucax
Copy link

llucax commented Jun 19, 2024

@pawamoy Not sure what the problem is now, but with griffe 0.47 I'm getting these errors:

WARNING -  mkdocs_autorefs: reference/frequenz/sdk/actor/index.md: Could not find cross-reference target '[frequenz.sdk.actor.Actor]'
WARNING -  mkdocs_autorefs: reference/frequenz/sdk/actor/index.md: Could not find cross-reference target '[frequenz.sdk.actor.Actor]'
WARNING -  mkdocs_autorefs: reference/frequenz/sdk/actor/index.md: Could not find cross-reference target '[frequenz.sdk.actor.Actor]'
WARNING -  mkdocs_autorefs: reference/frequenz/sdk/actor/index.md: Could not find cross-reference target '[frequenz.sdk.actor.run]'
WARNING -  mkdocs_autorefs: reference/frequenz/sdk/actor/index.md: Could not find cross-reference target '[frequenz.sdk.actor.run]'

While 0.45.3 and 0.46.1 work fine. Instructions to reproduce are the same as in #295.

@pawamoy
Copy link
Member

pawamoy commented Jun 19, 2024

Sorry for the trouble, let me look into it 👍

@pawamoy
Copy link
Member

pawamoy commented Jun 19, 2024

@llucax what version of mkdocstrings-python do you have?

@pawamoy
Copy link
Member

pawamoy commented Jun 19, 2024

OK, keeping only public objects was wrong, filtering out imported objects is wrong, the solution is to filter out imported objects unless they are public 🥵 Sorry for the confusion, the template logic is sometimes hard to get right. I'll prioritize the end-to-end tests PR next: mkdocstrings/python#157.

pawamoy added a commit to mkdocstrings/python that referenced this issue Jun 19, 2024
@pawamoy
Copy link
Member

pawamoy commented Jun 19, 2024

Released v1.8.5, it should behave correctly now.

@llucax
Copy link

llucax commented Jun 20, 2024

Yup, it is working again now. Thanks a lot for the super fast responses and fixes! 🎉

@pawamoy
Copy link
Member

pawamoy commented Jun 20, 2024

Thanks for testing new releases super early and sending bug reports 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unconfirmed This bug was not reproduced yet
Projects
None yet
Development

No branches or pull requests

4 participants