Revert "memoryview: remove inheritance from Sequence" - #12800
JelleZijlstra merged 2 commits into
Conversation
Sorry, something went wrong.
This reverts commit f625e92.
This comment has been minimized.
|
Is it possible to override the missing methods in a way that makes them unusable / almost always raise typing errors? Maybe something like a NoReturn + deprecated + Never args? |
Sorry, something went wrong.
|
Done. I set index and count to None as @AlexWaygood suggested in the other thread. I didn't attempt to set __contains__ and __reversed__ to None because while those specific methods don't exist on memoryview, the operations they represent do work. |
Sorry, something went wrong.
AlexWaygood
left a comment
There was a problem hiding this comment.
I like it much more with the None overrides, and your point about this causing unsound type narrowing with isinstance() is a good one. I still think you're underrating the impact of IDEs and autocomplete tools suggesting nonexistent "public-API" methods in autocompletions (and failing to error when the user accesses them in their code). But fixing it in CPython is clearly the correct long-term fix (thank you for taking up that cause!).
TL;DR: still not sure it's holistically better overall to have it inherit from Sequence, but fine with it being reverted, especially with the None overrides for index/count
Sorry, something went wrong.
|
Diff from mypy_primer, showing the effect of this PR on open source code: hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
- src/hydra_zen/structured_configs/_implementations.py:1137: error: No overload variant of "builds" of "BuildsFn" matches argument types "type[DataclassInstance]", "dict[str, int | float | memoryview[int] | Path | DataClass_ | <6 more items> | None]", "bool | None", "Literal['none', 'partial', 'all', 'object'] | None", "DataclassOptions" [call-overload]
+ src/hydra_zen/structured_configs/_implementations.py:1137: error: No overload variant of "builds" of "BuildsFn" matches argument types "type[DataclassInstance]", "dict[str, int | float | Path | DataClass_ | type[DataClass_] | ListConfig | DictConfig | Enum | Sequence[HydraSupportedType] | Mapping[Any, HydraSupportedType] | None]", "bool | None", "Literal['none', 'partial', 'all', 'object'] | None", "DataclassOptions" [call-overload]
- src/hydra_zen/structured_configs/_implementations.py:2666: error: Incompatible types in assignment (expression has type "tuple[int | float | memoryview[int] | Path | DataClass_ | <6 more items> | None, ...]", variable has type "list[Importable | Callable[P, R] | type[Builds[Importable]] | type[BuildsWithSig[Importable, Any]] | type[BuildsWithSig[type[R], P]] | Any]") [assignment]
+ src/hydra_zen/structured_configs/_implementations.py:2666: error: Incompatible types in assignment (expression has type "tuple[int | float | Path | DataClass_ | type[DataClass_] | ListConfig | DictConfig | Enum | Sequence[HydraSupportedType] | Mapping[Any, HydraSupportedType] | None, ...]", variable has type "list[Importable | Callable[P, R] | type[Builds[Importable]] | type[BuildsWithSig[Importable, Any]] | type[BuildsWithSig[type[R], P]] | Any]") [assignment]
|
Sorry, something went wrong.
Reverts #12781
Although it does not implement the full Sequence API at runtime, memoryview is in fact a subclass of Sequence at runtime (through ABC registration). I will work soon on changes in CPython to remedy the discrepancy, but that doesn't solve the problem on existing versions of Python.
I'd prefer to revert to the previous behavior where memoryview is a subclass of Sequence, for a few reasons: