-
Notifications
You must be signed in to change notification settings - Fork 204
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
fix docstring issues by adding an explicit handler #106
Conversation
Nice find ! Do you have any references to this? Because I knew this issue was in mkdocstrings but I couldn't really find this info. |
@aminalaee This is a pretty new tool so I had to do my own digging to find that. I ran The experimental handler uses another tool under the hood called "griffe" which has a CLI; you can type "griffe sqladmin" to see how it parses the package. |
I think it's possible to not add those attributes; by default it always includes docstring'd methods, but I think you can override that. I'll check later today. |
I'll open an issue on mkdocstrings and see how it goes, let's keep this open anyway. I think mkdocstrings is not wrong, we're making |
I do believe the legacy version (default handler) is wrong, and that the experimental version (the handler with The reason why it's wrong to use I think the more pressing issue is that the |
Well we haven't seen any progress with that issue and both approaches have their problems, I'd say let's comment out |
b2301ec
to
fe9b801
Compare
Two options. Both are fine:
|
fe9b801
to
50ac643
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR 🎉
Long story short: fixes #104.
Locking the dependency uses mkdocstrings's so-called "experimental" handler for docstring parsing. The new one properly parses the docstring in this case whereas the legacy one does not.