bpo-36669: Add matmul support for weakref.proxy #12932
Conversation
There was a problem hiding this comment.
Looks pretty straightforward. I assume __rmatmul__ is covered automatically by implementing __matmul__, but I didn't check. Either way I think adding a test for that would be good.
Also, it seems there is no pure python implementation of weakref.proxy? I checked Modules/weakref.py and didn't see one, so I assume that this is fine, but maybe I'm missing it. Just thinking of PEP 399.
| @@ -285,6 +285,18 @@ def __ifloordiv__(self, other): | |||
| p //= 5 | |||
| self.assertEqual(p, 21) | |||
|
|
|||
| def test_proxy_matmul(self): | |||
| class C: | |||
| def __matmul__(self, other): | |||
There was a problem hiding this comment.
Maybe add a test for __rmatmul__ as well?
|
@pganssle Thanks for reviewing! Re Python-only implementations, I think discussion of PEP 399-ifying of the |
Yes, I fully agree that it's out of scope. I only mentioned it because it seems like it's maybe partially compliant with PEP 399, so I wasn't sure if I was just missing a pure python implementation of |
|
One other question: Does this need a "What's New" entry, or a I think it's on the borderline in terms of changes that need to be in What's New / |
Yeah, I thought about this and couldn't decide. :-) I agree with your "borderline" comment. I'll go ahead and add a "whatsnew" entry; worst case is that whoever tidies up the 3.8 "what's new" deletes that entry as insignificant. I also couldn't decide whether to add the Misc/NEWS entry under the "Library" section or the "Core and Builtins" section: we're actually modifying a built-in Python object ( |
... and the |
weakref.proxyobjects currently support all numeric methods except the matrix multiplication operators (@and@=). This PR adds that missing support.https://bugs.python.org/issue36669