-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Finder found candidates cleanup #6415
Finder found candidates cleanup #6415
Conversation
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.
Thanks for the quick turnaround. Some comments.
src/pip/_internal/index.py
Outdated
@@ -288,22 +317,10 @@ def iter_all(self): | |||
|
|||
def iter_applicable(self): | |||
# type: () -> Iterable[InstallationCandidate] | |||
"""Iterate through candidates matching the given specifier. | |||
"""Iterate through candidates matching the given versions. |
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 would say something other than "given" here because the versions are associated with the object rather than given as arguments (which is what given normally means in the context of a function or method docstring).
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.
Is the current wording better? (I am not good at describing things.)
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.
It's not a big deal. But some other possibilities that come to mind: "Iterate through the candidates matching the instance's versions." or "matching the versions associated with the instance" or "matching the associated versions"
650d3ee
to
f499e11
Compare
f499e11
to
288f5b9
Compare
Changed the default specifier arg to None, and store versions as Set[str] instead. |
Good work! 👍Looks great! |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Follow-up cleaning mentioned in #5971