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

feat: include documentation for double-underscore methods when requested #646

Closed
wants to merge 1 commit into from
Closed

feat: include documentation for double-underscore methods when requested #646

wants to merge 1 commit into from

Conversation

gvwilson
Copy link

@gvwilson gvwilson commented Dec 9, 2023

  1. Add --include-dunder Boolean option to command line args to request that double-underscore (dunder) methods with docstrings be included.

  2. Store value as include_dunder in environment passed to Jinja2.

  3. Modify Jinja2 template to include dunder methods if option enabled.

  4. Add test for new feature.

This does not work as intended at the moment because inherited dunder methods with docstrings are displayed as well as those having dunder methods that are defined in the class. As a result, methods like __dict__ and __weakref__ show up for a class that only defines a docstring for __lt__ (and the docs for __eq__ also show up, even though the test explicitly does not have a docstring). Guidance would be greatly appreciated.

Closes #645

1.  Add `--include-dunder` Boolean option to command line args to request
    that double-underscore (dunder) methods with docstrings be included.

2.  Store value as `include_dunder` in environment passed to Jinja2.

3.  Modify Jinja2 template to include dunder methods if option enabled.

4.  Add test for new feature.

This does _not_ work as intended at the moment because inherited
dunder methods with docstrings are displayed as well as those having
dunder methods that are defined in the class. As a result, methods
like `__dict__` and `__weakref__` show up for a class that only
defines a docstring for `__lt__` (and the docs for `__eq__` also show
up, even though the test explicitly does *not* have a docstring).
Guidance would be greatly appreciated.

Closes #654
grscheller added a commit to grscheller/datastructures that referenced this pull request Dec 12, 2023
There is a pull request on the pdoc GitHub site:

  mitmproxy/pdoc#646

to include dunders if they have docstrings via

  pdoc --html --include-dunder grscheller.datastructures

This pull request is still a work in progress.
@mhils
Copy link
Member

mhils commented Dec 12, 2023

Note

For other folks stumbling on this, let me reiterate https://pdoc.dev/docs/pdoc.html#control-what-is-documented:

If you want to document a special __dunder__ method, the recommended way to do so is to not document the dunder method specifically, but to add some usage examples in the class documentation.

Thanks for filing #645 and sending an excellent PR along as well! 😃I generally remain in favor of not documenting dunders (and keeping them simple), but I guess there are legitimate use cases like yours. Let's see if we can enable that. Some thoughts:

  1. One thing I dislike about the current implementation is that it also covers all methods that start with an underscore, not just __dunder__ methods. That's definitely a bit surprising from a user perspective and would need to be fixed. Although I can already see a disappointed PR author for --include-private on the horizon. 😛
  2. As you pointed out the builtin docstrings for dunder methods are biting us here. We do already have a workaround for object.__init__ here, which could be extended at the cost of some additional complexity. I'm not a super big fan, but it's probably viable to compare against some getattr(object, attrname, None) shenanigans.
  3. An alternative approach would be to introduce a @public marker which can be added to docstrings, similar to how @private works. This has the advantage that users can be a bit more deliberate about which methods they want to document. The downside is that docstrings become slightly more noisy, but I feel that's fine. We would of course strip the marker in the rendered docs. The other difference is that this needs to be repeated for every method, but I feel maybe that's a good compromise because you shouldn't have a million dunder methods that need documentation after all?

Thoughts?

@gvwilson
Copy link
Author

Thank you for your feedback - I think @public is both the simplest and the least disruptive approach, particularly since this is a niche use case.

@mhils
Copy link
Member

mhils commented Jan 17, 2024

fixed in #655, thanks again!

@mhils mhils closed this Jan 17, 2024
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.

add --dunder flag to include documentation for double-underscore methods
2 participants