Thanks! Given how little mypy_primer fallout there is, I think we should probably go ahead with this! Rather than inventing a fake base class, though, could you just have memoryview inherit from Generic[_I]? It seems like the only method it inherits from its current superclasses without overriding the method is __reversed__, and it looks to me like it doesn't have a __reversed__ method at runtime either:

>>> memoryview.__reversed__
Traceback (most recent call last):
  File "<python-input-4>", line 1, in <module>
    memoryview.__reversed__
AttributeError: type object 'memoryview' has no attribute '__reversed__'

So I don't think the __memory_view class you added for it to inherit from should make any difference in practical terms

reversed(memoryview(b'')) is valid at runtime, does it still valid in typing after we remove __reversed__ ?

reversed(memoryview(b'')) is valid at runtime, does it still valid in typing after we remove __reversed__ ?

Yes. If you look at the stub, reversed doesn't actually only accept objects with __reversed__ methods; it can also accept any object that has a __getitem__ method and a __len__ method. memoryview satisfies this requirement, so type checkers will still accept it being passed to reversed() even after we remove the fictitious __reversed__ method:

thanks for your answer

thanks for your answer

No problem! Thanks for the PR :D

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 | 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: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: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]
+ 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]

This seems wrong:

>>> issubclass(memoryview, collections.abc.Sequence)
True

It may not implement the interface correctly, but it is registered as a subclass. I think we should reflect that subclass relationship in typeshed.

This seems wrong:

>>> issubclass(memoryview, collections.abc.Sequence)
True

It may not implement the interface correctly, but it is registered as a subclass. I think we should reflect that subclass relationship in typeshed.

I agree, but hot to accomplish this? is it possible to exclude some method from base classes?

This seems wrong:

>>> issubclass(memoryview, collections.abc.Sequence)
True

It may not implement the interface correctly, but it is registered as a subclass. I think we should reflect that subclass relationship in typeshed.

Ah, I should have checked that but forgot to; sorry. In this specific case I still think this change is a net improvement, given that:

• The fact the class is lacking 3 methods from the Sequence (index, count and __reversed__) is quite a serious deficiency in how the interface is implemented
• It may claim to be a subclass of Sequence at runtime, but neither collections.abc.Sequence nor typing.Sequence appears in the __mro__ at runtime
• The mypy_primer report indicates that very few people (well, nobody in the mypy_primer report, actually) are actually relying on type checkers understanding it as a subtype of Sequence and passing it to functions that accept arbitrary sequences
• We've had complaints in the past about type checkers incorrectly inferring ABCMeta as the metaclass for builtin types due to us having them inherit from generic collections in collections.abc/typing.

IMO we should take the issue of whether builtins should inherit from generic collections on a case-by-case basis, and in this case the benefits of the change seemed to clearly outweigh the costs. But I also don't feel too strongly if you want to revert. Another solution might be to put index = None in the stub for memoryview; type checkers would at least issue an error if you tried to use memoryview.index() then.

Is the mypy_primer trustworthy? Haven't yet thought it through but mypy still has the promotions behaviour and bytes is a Sequence

Is the mypy_primer trustworthy? Haven't yet thought it through but mypy still has the promotions behaviour and bytes is a Sequence

Oh, good question! Looks like pyright removed the promotion behaviour in microsoft/pyright#8835, so I suppose a primer run using pyright would be a better judge here.

Using AlexWaygood/pyright#1 for a primer run using pyright...

The mypy_primer report with pyright indicates it only adds one error in the checked projects, and the error occurs inside a pytest.raises() block: https://github.com/aio-libs/yarl/blob/d5660f3b34b0f9873eedfe6ef047eca6316f5ebe/tests/test_update_query.py#L317-L320, so it's meant to raise an exception anyway. I think that's okay -- WDYT?

Doesn't pyright primer run on fewer projects?

Intuitively I feel it's much likelier that someone is relying on memoryview being a Sequence than that they're relying on .index on arbitrary sequences and passing memoryviews.

I think we should also fix something in CPython here at runtime, though: either remove the Sequence inheritance from memoryview or add the missing methods at runtime.

Doesn't pyright primer run on fewer projects?

@hauntsaninja obviously knows more than me on that question! I only see 9 matches for pyright_cmd=None in https://github.com/hauntsaninja/mypy_primer/blob/master/mypy_primer/projects.py, though.

Intuitively I feel it's much likelier that someone is relying on memoryview being a Sequence than that they're relying on .index on arbitrary sequences and passing memoryviews.

well, but with the stubs prior to this, I imagine IDEs would be e.g. suggesting memoryview.index() in autocompletions for an object concretely typed as a memoryview, since memoryview inherited from Sequence and index is a concrete method on Sequence. But memoryview.index() just doesn't exist; the suggestion drawn from typeshed's stubs there would be flat-out wrong.

I think we should also fix something in CPython here at runtime, though: either remove the Sequence inheritance from memoryview or add the missing methods at runtime.

Yes, that would obviously be the ideal solution. I think it's pretty unlikely we'll be able to remove the ABC registration at runtime, though, for backwards-compatibility reasons, so adding the methods would probably be the only viable solution in CPython, I think.

I massively expanded the set of projects pyright runs on at some point this year. There are still differences in coverage, but none that really matter to us here (many fewer projects have pyright configuration, we don't necessarily check the exact same set of files, etc)