Skip to content

Sequence type does neither match the ABC nor the implementation of tuple #8345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
koehlma opened this issue Jan 30, 2020 · 3 comments
Closed

Comments

@koehlma
Copy link

koehlma commented Jan 30, 2020

Have a look at the following code:

import typing as t


seq1: t.Sequence[str] = ("Hello", "World!")
try:
    seq1.count(x="Hello")
except TypeError as error:
    print("Raises a `TypeError` anyway:", error)


VALUES: t.List[int] = [3, 4, 3, 5]


class MySequence(t.Sequence[int]):
    @t.overload
    def __getitem__(self, index: int) -> int:
        pass

    @t.overload
    def __getitem__(self, index: slice) -> t.List[int]:
        pass

    def __getitem__(self, index: t.Union[int, slice]) -> t.Union[int, t.List[int]]:
        return VALUES[index]

    def __len__(self) -> int:
        return len(VALUES)


seq2: t.Sequence[int] = MySequence()
try:
    seq2.count(x=3)
except TypeError as error:
    print("Raises a `TypeError` anyway:", error)

# does not raise a `TypeError` but does not type check
print(seq2.count(value=3))

The first two calls to count do type check but raise an error at runtime. The errors, however, are quite different: in the first case, count does not accept any keyword arguments as in case of a tuple the arguments are positional only. In the second case, count does accept keyword arguments but not x because the name of the argument is value in the ABC Sequence (which is in itself inconsistent, strictly speaking a tuple does not implement the Sequence ABC as it does not allow keyword arguments). Finally, the last call to count succeeds because it is valid according to the ABC but it does not type check.

The behavior I expect is as follows: with regard to the Sequence type, count should accept no keyword arguments. If no arguments are given using keywords, then tuple and the ABC are consistent.

I stumbled across this while trying to implement a protocol for hashable sequences. This turned out impossible because if I declare precisely the interface of the ABC then I cannot assign tuples to a variable having that type.

If you point me into a direction of where I should look to fix this, I might be able to send a pull request.

Python Version: 3.8.0
Mypy Version: 0.761

I use the following mypy configuration:

[mypy]
disallow_untyped_calls = True
disallow_untyped_defs = True
disallow_incomplete_defs = True
check_untyped_defs = True
disallow_subclassing_any = True
warn_no_return = True
strict_optional = True
strict_equality = True
no_implicit_optional = True
disallow_any_generics = True
disallow_any_unimported = True
warn_redundant_casts = True
warn_unused_ignores = True
warn_unused_configs = True
show_traceback = True
show_error_codes = True
pretty = True
@JelleZijlstra
Copy link
Member

I haven't thought too deeply about this, but it sounds like we should just mark the argument to Sequence.count as positional-only.

@koehlma
Copy link
Author

koehlma commented Jan 30, 2020

Yes, I think that this would be a proper fix.

It would also be great to make Sequence a proper protocol such that

T_co = t.TypeVar("T_co", covariant=True)

class HashableSequence(t.Hashable, t.Sequence[T_co], t.Protocol[T_co]):
    pass

would work. Currently this gives me

error: All bases of a protocol must be protocols  [misc]
    class HashableSequence(t.Hashable, t.Sequence[T_co], t.Protocol[T_co]):
    ^

because the ABC has concrete methods.

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 4, 2020

Many methods in the typing ABCs and protocols should perhaps have positional-only args. This would have to changed in typeshed (do you want to make a typeshed PR?), so I'm closing this issue here.

Making Sequence a protocol is a mostly unrelated matter, and would require CPython changes (and I'm not convinced that it's desirable, but again, this is not the best place to discuss it).

@JukkaL JukkaL closed this as completed Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants