-
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
Simplify FoundCandidates #6684
Simplify FoundCandidates #6684
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.
Me likey.
Thanks! |
self._candidates = candidates | ||
self._evaluator = evaluator | ||
self._versions = 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.
Late nit: I'd put an assert set(self._applicable_candidates) <= set(self._candidates)
to make sure args are coherent.
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 taking a look, @xavfernandez. I'll do this in a future PR.
This PR simplifies the
FoundCandidates
class by moving the calculation logic inFoundCandidates.iter_applicable()
into themake_found_candidates()
function that creates aFoundCandidates
object.This makes it so
FoundCandidates
behaves more like a simple container. It also makes it so that the determination of applicable candidates happens just once when theFoundCandidates
object is created, instead of each time one wants to iterate over the applicable candidates.This PR also adds the first test of the
make_found_candidates()
method.This PR will come in handy for some later PR's.