| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I knew that the numbers module was problematic, but I thought maybe this one would be okay. A single hit isn't the worst, but it's probably bad enough in this case? The potential upside is pretty small and the error is a false positive since Number isn't a real base of Decimal at runtime, just kinda-sorta a base maybe. While I'm here and for the record, the only other ABC registrations that occur in cpython but aren't currently represented in typeshed are all for classes from numbers: Complex.register(complex) Real.register(float) Integral.register(int) I believe that trying to get those into typeshed breaks approximately the entire world, if I understand the situation correctly. |
Sorry, something went wrong.
The last time I tried it locally I couldn't even get to the "let's file a PR and see what mypy_primer says" stage, since mypy just crashed on me :-) |
Sorry, something went wrong.
|
Look like it's only int that causes the crash. We should be able to see float and complex... |
Sorry, something went wrong.
|
I'm not a fan of adding "registered" base classes to the base class list anyway. This is one of those cases, where we hack around limitations of the type system, fixing one problem (isinstance checks), but potentially causing other problems. |
Sorry, something went wrong.
|
The primer output (and CI failures) also indicate another reasons why this change would be problematic: float and complex don't actually satisfy (at least, not on all Python versions) the abstract ABCs that are meant to describe float-like and complex-like classes (Real and Complex respectively). The fundamental reason why these ABCs are problematic is that they just don't make sense when it comes to static typing. It's impossible to fill in the missing annotations in these methods, because there's no way of doing so that would make them both suitable for flexible subclassing and would also accurately describe the stdlib runtime classes that are registered to them. Ultimately their use should just be discouraged for static typing purposes, so I think this PR goes in the wrong direction. (Really I think they need some kind of soft-deprecation notice in the docs, but that's a conversation we'd have to have over at CPython.) |
Sorry, something went wrong.
|
this PR goes in the wrong direction Yeah. To be clear there's nothing here that I'm advocating for at this point, but I'm still curious to see what it looks like if we tried. If it could be added to typeshed without causing problems I'd argue that we should add them even if actually using them in typing is a bad idea, but they can't be added without causing problems so it's a moot question. |
Sorry, something went wrong.
|
My notes on what gets surfaced: + tornado/ioloop.py:837: error: "IOLoop" has no attribute "_timeout_counter" [attr-defined]
+ xarray/core/variable.py: note: In member "_pad_options_dim_to_index" of class "Variable":
+ xarray/core/variable.py:1145: error: Unsupported target for indexed assignment ("Mapping[Any, int | float | tuple[int, int] | tuple[float, float]]") [index]
These are both new errors only because mypy previously considered their branch unreachable. Similarly, all the Unused "type: ignore" comment messages from core are because they're using both comparisons + tanjun/schedules.py:570: error: Subclass of "Sequence[int]" and "float" cannot exist: would have incompatible method signatures [unreachable] This is coming from code like: values: int | list[int] | None
if isinstance(values, int):
return
if isinstance(values, float):
raise TypeErrorMy guess is that there's some special casing of int and float that normally allows this but is broken by the change. error: Definition of "__neg__" in base class "Complex" is incompatible with definition in base class "_RealLike" [misc] error: Definition of "__pos__" in base class "Complex" is incompatible with definition in base class "_RealLike" [misc] These show up 6 times, making it the most common issue. In the one I checked, it showed for something like class foo: pass
class bar(float, foo): passDoesn't trigger for just class bar(float): pass. It's bad, certainly, and fixing it in typeshed would probably mean making the stubs worse (as Alex's comment describes), but it doesn't seem entirely insurmountable. error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases This one is reasonably unpleasant. It's a false positive that's The only part I don't quite understand yet is that for Decimal in this MR it shows up for code like class FooMeta(type): ...
class Foo(Decimal, metaclass=FooMeta): ...but for float that doesn't trigger it and it only shows up for code like class FooMeta(type): ...
class Bar(metaclass=FooMeta): ...
class Foo(float, Bar): ...All that said however, the same false positive already exists for classes registered to collections.abc classes: class FooMeta(type): ...
class Foo(list, metaclass=FooMeta): ... # error in mypy but not at runtime... |
Sorry, something went wrong.
|
Looks like Self doesn't turn up any problems? Is there a reason overrides were used instead of Self in #11375 ? Wouldn't work for __abs__ between _ComplexLike and _IntegralLike but it seems fine for __neg__ and __pos__ |
Sorry, something went wrong.
|
Diff from mypy_primer, showing the effect of this PR on open source code: tornado (https://github.com/tornadoweb/tornado)
+ tornado/ioloop.py:837: error: "IOLoop" has no attribute "_timeout_counter" [attr-defined]
ibis (https://github.com/ibis-project/ibis)
+ ibis/common/tests/test_patterns.py:1141: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
+ ibis/common/tests/test_grounds.py:191: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
+ ibis/common/tests/test_grounds.py:209: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
xarray (https://github.com/pydata/xarray)
+ xarray/core/variable.py: note: In member "_pad_options_dim_to_index" of class "Variable":
+ xarray/core/variable.py:1145: error: Unsupported target for indexed assignment ("Mapping[Any, int | float | tuple[int, int] | tuple[float, float]]") [index]
yarl (https://github.com/aio-libs/yarl)
+ tests/test_update_query.py:266:5: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
pydantic (https://github.com/pydantic/pydantic)
+ pydantic/v1/types.py:667: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
core (https://github.com/home-assistant/core)
+ homeassistant/util/unit_system.py:138: error: Unused "type: ignore" comment [unused-ignore]
+ homeassistant/util/unit_system.py:148: error: Unused "type: ignore" comment [unused-ignore]
+ homeassistant/util/unit_system.py:158: error: Unused "type: ignore" comment [unused-ignore]
+ homeassistant/util/unit_system.py:168: error: Unused "type: ignore" comment [unused-ignore]
+ homeassistant/util/unit_system.py:178: error: Unused "type: ignore" comment [unused-ignore]
+ homeassistant/helpers/config_validation.py:259: error: Unused "type: ignore" comment [unused-ignore]
|
Sorry, something went wrong.
|
I spent a little time with the mypy crash. It happens on top-level annotations of FOO: Literal[2] because builtins.int isn't fully resolved yet and there's no accommodation for that possibility. I tried letting that section of the type analyzer defer, and after that I could get mypy to run successfully on a fully numbers-ized version of typeshed.
Fixing all those got me to a successful stubtest run though! Probably time to leave well enough alone after that, but it would be fun to see the primer output at that point. |
Sorry, something went wrong.
|
python/mypy#18046 to fix the mypy crash, if that's a thing mypy wants to do |
Sorry, something went wrong.
|
Nothing else to do here until/unless mypy accepts python/mypy#18046 and we can see what happens for full numberization. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Speaking of ABC registrations within the standard library, here's one that does happen. Unless I missed something in my search, it's one of only two in cpython to register to an ABC from within C (the other is array.array getting registered to MutableSequence).