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

bpo-30140: fix binop dispatch for subclasses #1325

Closed
wants to merge 2 commits into from

Conversation

shoyer
Copy link
Contributor

@shoyer shoyer commented Apr 27, 2017

This doesn't work yet, but I'm not quite sure why. This is now working.

https://bugs.python.org/issue30140

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

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 we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@mention-bot
Copy link

@shoyer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @serhiy-storchaka, @benjaminp and @nascheme to be potential reviewers.

@shoyer shoyer changed the title [WIP] bpo-30140: fix binop dispatch for subclasses bpo-30140: fix binop dispatch for subclasses Sep 8, 2017
self.assertEqual(E() // 1, "C.__floordiv__")
self.assertEqual(1 // E(), "C.__rfloordiv__")
self.assertEqual(E() // C(), "C.__floordiv__")
self.assertEqual(C() // E(), "C.__floordiv__") # This one would fail

Choose a reason for hiding this comment

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

Why are these being taken out? I don't see div or floordiv in the above, so it doesn't look like it is just being tested differently (unless the tests were really redundant or something).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests here seemed redudant with the tests I added for add above, since the logic is not operator specific. I removed these specific testa because they were verifying the old (changed) behavior, but I could also keep them in and change to the new behavior.

@gvanrossum
Copy link
Member

Let's not merge until the new BDFL has decided on https://bugs.python.org/issue30140#msg325553

@alimcmaster1
Copy link

Looks like there is little appetite to merge/make a decision on this, Good to close?

@gvanrossum
Copy link
Member

Yeah, let's give up on this one.

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

Successfully merging this pull request may close these issues.

None yet

8 participants