-
Notifications
You must be signed in to change notification settings - Fork 156
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
General typing improvement #610
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #610 +/- ##
==========================================
- Coverage 71.28% 71.00% -0.28%
==========================================
Files 13 13
Lines 1069 1083 +14
==========================================
+ Hits 762 769 +7
- Misses 307 314 +7 ☔ View full report in Codecov by Sentry. |
There is no public interface, and the README says as much. See #262 for a discussion on that. |
Fair enough, bad choice of words on my part. I used the word “public” because I’ve recently seen a few uses of I figured improving the typing overall might make such usage a bit easier while not affecting anything the wheel package actually does. |
Even if all this provides is a few quality of life improvements for typing users who have |
Just following up, @agronholm: Would you consider this PR, even if it doesn't change functionality at all? Might be unnecessary, but to make my case again: It's more like documentation or typing aid for typing users that use wheel at runtime (even without a public interface/API). As previously mentioned, it doesn't seem uncommon to use Hope my reasons makes sense, and I'm more than willing to take feedback if I'm wrong. |
Well, so long as we only use these annotations to sort of improve the internal documentation, and don't advertise them by adding a |
More than fair, thank you. If there's anything else I can do, don't hesitate to let me know. |
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.
I added some missing annotations, and commented on the pyright config.
Add a few more annotations and remove pyright config. Co-authored-by: Alex Grönholm <[email protected]>
Thanks. |
No problem! Thanks for considering it. |
Essentially just adds a few more types to the public interface and some of the internal interface. I tried to avoid any changes to logic or configuration, though I added pyright configuration to pyproject.toml to aid these additions. That section can easily be removed.
Upon running tox, all tests seem to pass.