close
Skip to content

bpo-27575: port set intersection logic into dictview intersection#7696

Merged
rhettinger merged 15 commits intopython:masterfrom
fgregg:dictview_intersection
Aug 26, 2019
Merged

bpo-27575: port set intersection logic into dictview intersection#7696
rhettinger merged 15 commits intopython:masterfrom
fgregg:dictview_intersection

Conversation

@fgregg
Copy link
Copy Markdown
Contributor

@fgregg fgregg commented Jun 14, 2018

This turns a patch contributed by @dsu1995 on bpo into a github PR.

The patch ports the logic of set intersection into the intersection of dictviews. It will address performance issues when taking the intersection of large dictviews.

https://bugs.python.org/issue27575

@the-knights-who-say-ni
Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

When your account is ready, please add a comment in this pull request
and a Python core developer will remove the CLA not signed label
to make the bot check again.

Thanks again for your contribution, we look forward to reviewing it!

@fgregg
Copy link
Copy Markdown
Contributor Author

fgregg commented Jun 15, 2018

@rhettinger, this PR is ready for review. I've verified all the code paths are covered by the tests.

@Mariatta
Copy link
Copy Markdown
Member

Friendly reminder to use "Co-authored by" in the commit message in situations like this, where patch was originally started by someone else. Thanks.

Devguide reference
GitHub article

@fgregg
Copy link
Copy Markdown
Contributor Author

fgregg commented Jun 16, 2018

Hi @Mariatta!

I wasn't sure if that guidance applied here, because I didn't apply @dsu1995 patches and them make a new commit. Instead, I cherry-picked from his git branch.

As such, David Su is already listed as the sole author of his commits.

If this is not the preferred option, I can redo this branch so that it does not have @dsu1995's commits and instead just has a commit where I applied his patch. In that commit I will list him as the Co-Author in the commit message.

Please advise.

@methane methane self-requested a review April 11, 2019 14:02
@bedevere-bot
Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@csabella
Copy link
Copy Markdown
Contributor

@fgregg, please address the code review requests. Also, due to the code review requests, the 'Co-authored by:' seems appropriate.

@rhettinger rhettinger merged commit 998cf1f into python:master Aug 26, 2019
@fgregg
Copy link
Copy Markdown
Contributor Author

fgregg commented Sep 3, 2019

thanks for finishing this up, @rhettinger! And for the opportunity to contribute to CPython

@dsu1995
Copy link
Copy Markdown

dsu1995 commented Sep 3, 2019

Thank you @rhettinger! And thank you @fgregg for migrating my bpo patch to a Github PR and cleaning up the code!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants