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

Support PEP-0561 compatible stub-only packages #671

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

ErikDeSmedt
Copy link
Contributor

Add support for stub-only package.
The typehints for the package flyingcircus could be in the flyingcircus-stubs package.

@ErikDeSmedt
Copy link
Contributor Author

I've renamed the dir variable to path_dir.
There is no impact in this case. However, overriding python built-ins makes me nervous

Copy link
Member

@mhils mhils 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 the PR! Could you please add a CHANGELOG entry as well? :)

pdoc/doc_pyi.py Show resolved Hide resolved
pdoc/doc_pyi.py Outdated
module_part_name[0] = f"{module_part_name[0]}-stubs"
module_stub_path = "/".join(module_part_name)

for path_dir in sys.path:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for path_dir in sys.path:
for search_dir in sys.path:

maybe? Strong +1 to not overriding builtins, but path in the variable name is typically a Path object. I'd be equally happy with just d.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Add support for stub-only package.
The typehints for the package `flyingcircus` could be in the
`flyingcircus-stubs` package.
@ErikDeSmedt
Copy link
Contributor Author

Thanks for the quick review.
I've pushed a new version that should address all comments

Copy link
Member

@mhils mhils left a comment

Choose a reason for hiding this comment

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

Thanks again! 🍰

@mhils mhils merged commit f75638f into mitmproxy:main Feb 28, 2024
12 checks passed
ErikDeSmedt added a commit to ErikDeSmedt/greenlight that referenced this pull request Mar 4, 2024
We use pdoc to generate a reference of the python `glclient`-library.
We suffered some issues related to missing type-hints. The work-around
was to remove the type-hints before generating the docs.

I've found 3 causes of warnings.

1. Missing export of `SignerHandle` (See Blockstream#368)
2. Poorly defined imports in auto-generated GRPC-code (See
   Blockstream#369)
3. pdoc failing to import type from type-stubs
   (See mitmproxy/pdoc#671)

All of these 3 issues have been addressed so we don't need the hack
anymore.
cdecker pushed a commit to ErikDeSmedt/greenlight that referenced this pull request Jun 6, 2024
We use pdoc to generate a reference of the python `glclient`-library.
We suffered some issues related to missing type-hints. The work-around
was to remove the type-hints before generating the docs.

I've found 3 causes of warnings.

1. Missing export of `SignerHandle` (See Blockstream#368)
2. Poorly defined imports in auto-generated GRPC-code (See
   Blockstream#369)
3. pdoc failing to import type from type-stubs
   (See mitmproxy/pdoc#671)

All of these 3 issues have been addressed so we don't need the hack
anymore.
cdecker pushed a commit to Blockstream/greenlight that referenced this pull request Jun 7, 2024
We use pdoc to generate a reference of the python `glclient`-library.
We suffered some issues related to missing type-hints. The work-around
was to remove the type-hints before generating the docs.

I've found 3 causes of warnings.

1. Missing export of `SignerHandle` (See #368)
2. Poorly defined imports in auto-generated GRPC-code (See
   #369)
3. pdoc failing to import type from type-stubs
   (See mitmproxy/pdoc#671)

All of these 3 issues have been addressed so we don't need the hack
anymore.
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.

2 participants