Skip to content
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

New Resolver: Implement equality on candidate classes #7976

Merged
merged 2 commits into from
Apr 10, 2020

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented Apr 4, 2020

Hopefully this one is self-explainatory. RequiresPythonCandidate does not need this because there will always be exactly one instance, so the built-in object.__eq__ is sufficient. Or maybe it would be more obvious if we implement it anyway with self is other?

@uranusjr uranusjr requested review from pfmoore and pradyunsg April 4, 2020 07:20
@uranusjr uranusjr added the skip news Does not need a NEWS file entry (eg: trivial changes) label Apr 4, 2020
@pfmoore
Copy link
Member

pfmoore commented Apr 4, 2020

Maybe just add a comment to the RequiresPythonCandidate class explaining that built in __eq__ is enough? I see no reason to actually implement the default behaviour explicitly.

@uranusjr
Copy link
Member Author

uranusjr commented Apr 4, 2020

Sounds reasonable, added.

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Apr 8, 2020
@uranusjr uranusjr force-pushed the installed-candidate-equal branch from 6ff8dd8 to 591d476 Compare April 8, 2020 08:38
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Apr 8, 2020
@uranusjr uranusjr changed the title Implement equality on candidate classes New Resolver: Implement equality on candidate classes Apr 9, 2020
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

Looks great!

return self.base == other.base and self.extras == other.extras
return False

# Needed for Python 2, which does not implement this by default
Copy link
Member

Choose a reason for hiding this comment

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

I legit don't wanna wait.

@pradyunsg pradyunsg merged commit 8cca170 into pypa:master Apr 10, 2020
@uranusjr uranusjr deleted the installed-candidate-equal branch April 10, 2020 14:01
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation skip news Does not need a NEWS file entry (eg: trivial changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants