bpo-36669: Add matmul support for weakref.proxy#12932
bpo-36669: Add matmul support for weakref.proxy#12932mdickinson merged 5 commits intopython:masterfrom
Conversation
pganssle
left a comment
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.
|
|
||
| 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