Skip to content

Conversation

gvanrossum
Copy link
Member

Instead define abstract len in affected classes.

Fixes #2655 without breaking
rominf/ordered-set-stubs#1

Instead define abstract __len__ in affected classes.

Fixes #2655 without breaking
rominf/ordered-set-stubs#1
@gvanrossum gvanrossum requested a review from JukkaL November 30, 2018 17:13
def __len__(self) -> int: ...

class Sequence(_Collection[_T_co], Reversible[_T_co], Generic[_T_co]):
class Sequence(Iterable[_T_co], Container[_T_co], Reversible[_T_co], Generic[_T_co]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe preserve _Collection here, as otherwise mypy could infer a less precise type for some type joins? Then you could leave out __len__ as it would be inherited.

@srittau
Copy link
Collaborator

srittau commented Nov 30, 2018

You declare __len__() to be abstract in this patch. While this looks correct to me, I wonder why the other methods in those types are not declared to be abstract. Should we add @abstractmethod to those (in a separate PR), is @abstractmethod wrong for __len__() as well or is there a reason to treat them differently?

@JukkaL
Copy link
Contributor

JukkaL commented Nov 30, 2018

Some of the ABCs define various concrete methods that can act as default implementations.

@gvanrossum gvanrossum merged commit de50614 into master Nov 30, 2018
@gvanrossum gvanrossum deleted the expunge-sized-from-typing branch November 30, 2018 21:10
gvanrossum added a commit to python/mypy that referenced this pull request Nov 30, 2018
gvanrossum added a commit to python/mypy that referenced this pull request Nov 30, 2018
yedpodtrzitko pushed a commit to yedpodtrzitko/typeshed that referenced this pull request Jan 23, 2019
Instead define abstract __len__ in affected classes.

Fixes python#2655 without breaking
rominf/ordered-set-stubs#1
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

Successfully merging this pull request may close these issues.

3 participants