Skip to content

Binding an abc to TypeVar loses abstract class check #1830

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
euresti opened this issue Jul 8, 2016 · 4 comments
Closed

Binding an abc to TypeVar loses abstract class check #1830

euresti opened this issue Jul 8, 2016 · 4 comments

Comments

@euresti
Copy link
Contributor

euresti commented Jul 8, 2016

I was looking at the docstring of Type and noticed that it used a TypeVar for the argument. I wasn't using TypeVars so I started adding them to my code. But then I noticed I was getting fewer errors and mypy was not warning me about creating abstract base classes. Here's my simplified code:

from abc import ABCMeta, abstractmethod
from six import add_metaclass
from typing import TypeVar, Type

@add_metaclass(ABCMeta)
class User(object):
    @abstractmethod
    def foo(self):
        pass

class BasicUser(User):
    def foo(self):
        return 'yay'

try:
    u = User()   # This fails correctly.
except TypeError:
    pass
basic = BasicUser()


# Using a TypeVar
U = TypeVar('U', bound=User)
def new_user(user_class):
    # type: (Type[U]) -> U
    user = user_class()   # This passes typechecking.
    return user

joe = new_user(BasicUser)
try:
    jo2 = new_user(User)   # This should probably fail but doesn't.
except TypeError:
    pass


# Not using a TypeVar
def new_user2(user_class):
    # type: (Type[User]) -> User
    user = user_class()    # This fails typecheck.
    return user

jar = new_user2(BasicUser)
try:
    ja2 = new_user2(User)
except TypeError:
    pass

(This was in python 2)

I can't tell what's better. It's annoying that (Type[User]) -> User wouldn't work. But I sort of understand that it can't guarantee to work. What I've done to work around this is have user_class instead be a function that returns a User.

def new_user3(user_class_fn):
    # type: (Callable[[], User]) -> User
    user = user_class_fn()
    return user

def get_basic_user():
    # type: () -> User
    return BasicUser()

def get_user():
    # type: () -> User
    return User()  # this fails correctly

jar = new_user3(get_basic_user)
try:
    ja2 = new_user3(get_user)
except TypeError:
    pass
@JukkaL
Copy link
Collaborator

JukkaL commented Jul 8, 2016

See #1831 for a proposed change that could perhaps address this problem as well.

@gvanrossum
Copy link
Member

Could we perhaps make it so that if you have def foo(a: Type[A]) where A is an abstract class, you are only allowed to call it with a concrete subclass?

@gvanrossum gvanrossum removed this from the Future milestone Mar 29, 2017
@elazarg
Copy link
Contributor

elazarg commented Sep 24, 2017

Support for add_metaclass() has been added in #3842.

Line 31 now fails

    jo2 = new_user(User)  # E: Only concrete class can be given where "Type[User]" is expected

Line 44 also fails:

    ja2 = new_user2(User)   # E: Only concrete class can be given where "Type[User]" is expected

No error is reported for new_user2

def new_user2(user_class):
    # type: (Type[User]) -> User
    user = user_class()
    return user

Looks like this bug has been resolved. Am I wrong?

@gvanrossum
Copy link
Member

Yeah, this has been fixed -- I recall seeing a PR, but I don't recall the details. If you want to be really thorough you can use git bisect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants