Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: fix stub merging for single-file packages #78

Merged
merged 9 commits into from
Jun 25, 2022

Conversation

tlambert03
Copy link
Contributor

closes #77

pyi stub files were not being detected and merged in two cases:

  1. single-module packages, where a .pyi file is also distributed and present in site-packages
  2. top module __init__.pyi. This is probably more common than the first case.

src/griffe/loader.py Outdated Show resolved Hide resolved
src/griffe/finder.py Outdated Show resolved Hide resolved
src/griffe/dataclasses.py Outdated Show resolved Hide resolved
@pawamoy
Copy link
Member

pawamoy commented May 31, 2022

Same comment as on your other PR: will do my best to review soon, and thank you for your work!

@pawamoy pawamoy self-requested a review May 31, 2022 16:47
@tlambert03
Copy link
Contributor Author

don't hurry on my behalf! I know exactly how it goes :)

.gitignore Show resolved Hide resolved
src/griffe/dataclasses.py Outdated Show resolved Hide resolved
src/griffe/finder.py Outdated Show resolved Hide resolved
src/griffe/finder.py Outdated Show resolved Hide resolved
src/griffe/loader.py Outdated Show resolved Hide resolved
@tlambert03
Copy link
Contributor Author

ok, I moved the stub finding logic outside the finder. It will continue to fail to find stubs for which an actual .py file doesn't exist, but will at least eliminate bug that motivated #77 for me.

@pawamoy
Copy link
Member

pawamoy commented Jun 23, 2022

Hey @tlambert03, I'm back 🙂 Busy weeks, sorry.

So, I wanted to try another approach first, and I did, but eventually I like what you did here. I still mixed a bit of both approach, I'll push directly on this PR so you can see, if that's OK with you!

@tlambert03
Copy link
Contributor Author

Hey @tlambert03, I'm back 🙂 Busy weeks, sorry.

no worries, welcome back :)

I still mixed a bit of both approach, I'll push directly on this PR so you can see, if that's OK with you!

awesome. yes push away!

@pawamoy
Copy link
Member

pawamoy commented Jun 25, 2022

So! I've pushed my changes.

Here's an explanation:

  • merged "set members" mixins to bring the same functionality for both modules and collections, without spaghetti code
  • in the finder, find_package does all the work of returning the right wrapper Package or NamespacePackage
  • Package gained a stubs attribute
  • the two previous points allow us to handle top modules along with their stubs, even when submodules=False. I've previously tried to do it differently but it didn't take submodules into account (btw I'm starting to doubt the usefulness of submodules=False...)
  • additional test for modules with stubs directly in a search path

Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

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

Let me know what you think! Maybe we could add comments to document the parts that are difficult to understand?

src/griffe/mixins.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@tlambert03 tlambert03 left a comment

Choose a reason for hiding this comment

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

Yeah, looks great! I like the changes. This is good by me 👍

@pawamoy
Copy link
Member

pawamoy commented Jun 25, 2022

Great, thank you! I'll merge and cut a new release 🙂

@pawamoy pawamoy merged commit 6a82542 into mkdocstrings:master Jun 25, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: support type stubs?
2 participants