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

added support for nested classes; this is basically a newer version o… #390

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

frank101010
Copy link
Contributor

@frank101010 frank101010 commented Mar 29, 2022

Adds support for nested classes to pdoc3.

This is basically a newer version of angeloskath's commit 467350c, #176, but since I need support for nested classes, and the original merge request hasn't changed status since June 2020, I decided to apply most of angeloskath's changes into a branch derived from the most recent master.

(deleted section about unit test problems, which were resolved after running tests on an os supporting symbolic links)

Copy link
Member

@kernc kernc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing this up again!

Would you care to add a small unit test to show this new feature works as advertised and so we don't break it later?

pdoc/__init__.py Outdated Show resolved Hide resolved
pdoc/__init__.py Outdated Show resolved Hide resolved
pdoc/__init__.py Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@frank101010
Copy link
Contributor Author

frank101010 commented Apr 6, 2022

@kernc: you might want to wait with the next review, I've discovered a problem regarding warnings being issued, which I've already resolved. But there are also cases where member types are not properly rendered as hyperlinks in html output. The latter problem is due to the usage of inspect.formatannotations(), which does not format built-in container types like dict[str, int] properly (works with typing.Dict[str. int], though); but I'm going to create a new pull request for this.

@frank101010
Copy link
Contributor Author

@kernc: I just pushed new changes which fix a problem nested classes being recognized as pdoc.External instead of pdoc.Class. This required adding the nested classes into pdoc.Module's context. Unfortunately, this also breaks one unit test, namely ApiTest.test__all__. Since I don't really understand how that test works, could you please have a look at it?

@kernc
Copy link
Member

kernc commented Apr 6, 2022

The test tries to ensure that only the identifiers in __all__ are exposed. Interestingly, I'm not sure where the value 'B.B.C' comes from. I'd understand 'B.C' since that's what's nested. 🤔

class C:
"""B.C docstring"""

pdoc/test/__init__.py Outdated Show resolved Hide resolved
pdoc/test/__init__.py Outdated Show resolved Hide resolved
pdoc/__init__.py Outdated Show resolved Hide resolved
pdoc/__init__.py Outdated Show resolved Hide resolved
pdoc/__init__.py Outdated Show resolved Hide resolved
@JPHutchins
Copy link

@frank101010 @kernc Is this good to go? I require this functionality and can work on the PR by reopening on my own fork if you don't have time.

Thanks,
JP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants