collections: Add missing reflected BinOp methods - #7207
JelleZijlstra merged 4 commits into
Conversation
Sorry, something went wrong.
This comment has been minimized.
| def __mul__(self: Self, n: int) -> Self: ... | ||
| def __rmul__(self: Self, n: int) -> Self: ... | ||
| def __mod__(self: Self, args: Any) -> Self: ... | ||
| def __rmod__(self: Self, template: object) -> Self: ... |
There was a problem hiding this comment.
This one is actually broken:
In [12]: [] % collections.UserString("x")
Traceback (most recent call last):
File "/main_instance_shell/jelle/venv/lib/python3.6/site-packages/IPython/core/interactiveshell.py", line 3343, in run_code
exec(code_obj, self.user_global_ns, self.user_ns)
File "<ipython-input-12-0ca5ac41c5a2>", line 1, in <module>
[] % collections.UserString("x")
File "/main_instance_shell/jelle/venv/lib/python3.6/collections/__init__.py", line 1162, in __rmod__
return self.__class__(format % args)
NameError: name 'args' is not defined
It should error but not with a NameError.
Sorry, something went wrong.
There was a problem hiding this comment.
Re the NameError, do you wanna file the BPO issue or shall I? :)
Sorry, something went wrong.
There was a problem hiding this comment.
Go ahead! I think this method just shouldn't exist. It's presumably there to support %-formatting, but I don't think a reflected method makes sense for that.
Sorry, something went wrong.
There was a problem hiding this comment.
Wait, I can't reproduce the NameError. This is what I get on my machine:
>>> from collections import UserString
>>> [] % UserString('foo')
'[]'
>>> [] % UserString('x')
'[]'
>>>
Sorry, something went wrong.
There was a problem hiding this comment.
Ah, it got fixed in https://bugs.python.org/issue25652. That's what I get for trying things out on 3.6. I admit I don't understand the reasoning they ended up with for keeping the method.
Sorry, something went wrong.
There was a problem hiding this comment.
collections.UserString does feel like it was written during a phase when Raymond Hettinger was incredibly hyped about Javascript or something:
>>> 5 + UserString('foo')
'5foo'
>>> NotImplemented + UserString('foo')
'NotImplementedfoo'Like... wut...
Sorry, something went wrong.
There was a problem hiding this comment.
In [17]: collections.UserString("x") + collections.UserList(["y"])
Out[17]: "x['y']"
In [18]: collections.UserList(["y"]) + collections.UserString("x")
Out[18]: ['y', 'x']
Sorry, something went wrong.
There was a problem hiding this comment.
Truly a nightmare of what Python could have been
Sorry, something went wrong.
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
This comment has been minimized.
|
The mypy-primer errors are here: https://github.com/sphinx-doc/sphinx/blob/4.x/sphinx/locale/__init__.py#L67. They're already type-ignoring similar issues with __add__, so it seems fine to produce the same error on __radd__. |
Sorry, something went wrong.
|
Diff from mypy_primer, showing the effect of this PR on open source code: sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/locale/__init__.py: note: In member "__radd__" of class "_TranslationProxy":
+ sphinx/locale/__init__.py:70:5: error: Argument 1 of "__radd__" is incompatible with supertype "UserString"; supertype defines the argument type as "object"
+ sphinx/locale/__init__.py:70:5: note: This violates the Liskov substitution principle
+ sphinx/locale/__init__.py:70:5: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
+ sphinx/locale/__init__.py:70:5: error: Return type "str" of "__radd__" incompatible with return type "_TranslationProxy" in supertype "UserString"
+ sphinx/locale/__init__.py: note: In member "__rmod__" of class "_TranslationProxy":
+ sphinx/locale/__init__.py:76:5: error: Return type "str" of "__rmod__" incompatible with return type "_TranslationProxy" in supertype "UserString"
+ sphinx/locale/__init__.py: note: In member "__rmul__" of class "_TranslationProxy":
+ sphinx/locale/__init__.py:82:5: error: Return type "str" of "__rmul__" incompatible with return type "_TranslationProxy" in supertype "UserString"
|
Sorry, something went wrong.
Testing confirms that mypy currently raises false positives for these operations, whereas they all succeed at runtime.