-
Notifications
You must be signed in to change notification settings - Fork 259
improve type hint for Package.files
#904
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideTightens the type hint for Package.files by introducing a dedicated TypedDict (PackageFile) describing the expected file and hash keys, and updating the Package.files attribute to use this more precise type. Class diagram for tightened typing of Package.filesclassDiagram
class PackageFile {
+str file
+str hash
}
class Package {
+Sequence~PackageFile~ files
}
Package "1" o-- "*" PackageFile
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey - I've left some high level feedback:
- Consider whether
PackageFileshould declare any optional keys (e.g. usingTypedDict(total=False)orNotRequired[...]) if real-worldself.filesentries sometimes omitfileorhash, otherwise this stricter type may not accurately reflect existing usage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider whether `PackageFile` should declare any optional keys (e.g. using `TypedDict(total=False)` or `NotRequired[...]`) if real-world `self.files` entries sometimes omit `file` or `hash`, otherwise this stricter type may not accurately reflect existing usage.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
Yeah, but since it is poetry-core we had to vendor it. I tend to think that vendoring is not worth it just for this. |
e55ecb0 to
5482e60
Compare
It is a matter of weighing things up but I believe checking the keys is worth more than checking immutability.
See python-poetry/poetry#10673 for issues that the improved typing uncovered.
Summary by Sourcery
Improve typing of package file metadata on packages.
Enhancements: