Skip to content
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

Defaults for Generics? #4236

Closed
srittau opened this issue Nov 10, 2017 · 16 comments
Closed

Defaults for Generics? #4236

srittau opened this issue Nov 10, 2017 · 16 comments

Comments

@srittau
Copy link
Contributor

srittau commented Nov 10, 2017

Consider the following (simplified) typeshed definition for email.feedparser.FeedParser:

class FeedParser:
    def __init__(self, _factory: Callable[[], Message] = ...) -> None: ...
    def close(self) -> Message: ...

FeedParser allows a message factory to be passed in and will use it to construct the return value to close(). Unfortunately you have to cast the return value of close() to the correct type:

parser = FeedParser(MyMessage)
message = cast(MyMessage, parser.close())

The following type definition fixes this problem:

M = TypeVar("M", bound=Message)

class FeedParser(Generic[M]):
    def __init__(self, _factory: Callable[[], M] = ...) -> None: ...
    def close(self) -> M: ...

Now the example above does not require the cast to make mypy happy. But unfortunately using the default factory will not work anymore:

parser = FeedParser()
message = parser.close()

This will cause mypy to complain error: Need type annotation for variable. Is there any solution for this conundrum?

@emmatyping
Copy link
Collaborator

#4227 is somewhat related, but the underlying issue is that since mypy doesn't know what the return type of the _factory() Callable is (since the default is of the uninhabited type), mypy cannot deduce the return type to close().

It seems to me we might be able to fall back to the bound type, but I'm not sure about that.

@srittau
Copy link
Contributor Author

srittau commented Nov 10, 2017

Would having a "default" type for TypeVars make sense? Falling back to the bound type makes sense with my example above, but I am also unsure whether that is the right thing to do for the general case.

@chadrik
Copy link
Contributor

chadrik commented Nov 13, 2017

I've run into a similar problem, where I intuitively wanted to add a default value in my stubbed function to affect the outcome:

M = TypeVar('M', bound=Mapping)

def asdict(inst: Any, recurse: bool = ..., dict_factory: M = dict) -> M: ...
reveal_type(asdict(foo))  # should be `dict`

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 15, 2017

There seem to be two separate things going on here:

  • Infer a default value for type variable from the type of a default argument value (@chadrik's example above). I've wanted this once or twice. Overloading seems like a reasonable workaround, though.
  • Give an explicit default value for a type variable. The scope could be for all instances of type variable or just for a particular class/function.
    • If it's for all instances, we may need a change to TypeVar, such as T = TypeVar('T', default=str). This needs to be proposed at https://github.com/python/typing.
    • If it's for a particular definition, we could use a class/function decorator such as @mypy_extensions.type_var_default(T, str). This could well be mypy-specific.

I think that the first change is not very controversial, but pretty low priority since there is a workaround. For the second change we'd need more evidence of it being generally useful.

@chadrik
Copy link
Contributor

chadrik commented Nov 15, 2017

I think that the first change is not very controversial, but pretty low priority since there is a workaround.

I can agree with this as long as this use case is taken into consideration during the overflow overhaul (henceforth known as the overflowhaul). Currently I can't solve this without running into overlapping overload errors.

@srittau
Copy link
Contributor Author

srittau commented Nov 15, 2017

If I understand correctly, the first suggestion would also allow us to write the FeedParser definition as:

M = TypeVar("M", bound=Message)

class FeedParser(Generic[M]):
    def __init__(self, _factory: Callable[[], M] = Message) -> None: ...
    def close(self) -> M: ...

I think that's a fine solution.

@ilevkivskyi
Copy link
Member

@JukkaL

For the second change we'd need more evidence of it being generally useful.

It looks like this pattern is used extensively in SQLAlchemy. A simplified example:

class Null: ...

class Clause(Generic[T]):
    def __init__(self, type: Optional[Type[T]] = None) -> None:
        if type is None:
            type = Null  # Something like this happens at runtime
        self.type = type

reveal_type(Clause(int))  # OK, `Clause[int]`
reveal_type(Clause())  # Bad, `Clause[<nothing>]`, should be `Clause[Null]`

A potential solution is to use an overloaded __new__ but unfortunately it doesn't work.

I think this can be raised to at least normal priority.

@bryanforbes
Copy link
Contributor

IMO, I think the T = TypeVar('T', default=str) syntax would be the most robust in the long-run. TypeScript has a similar syntax and it's very handy:

class Clause<T = Null> {
    type: T;

    constructor(type?: T) {
        this.type = type ? type() : Null();
    }
}

let c = new Clause()
c.type // type is Null
let d = new Clause(Integer);
d.type // type is Integer

This could also potentially be combined with the bound parameter of TypeVar() to provide both the base type of the generic and a default value. @ilevkivskyi's example above would look like this:

class Null: ...

T = TypeVar('T', default=Null)
class Clause(Generic[T]):
    type: T
    def __init__(self, , type: Optional[Type[T]] = None) -> None:
        if type is None:
            type = Null  # Something like this happens at runtime
        self.type = type()

reveal_type(Clause(int))  # OK, `Clause[int]`
reveal_type(Clause()) # Ok, `Clause[Null]`

Then a developer (of either stubs or a library) wouldn't need to mess with __new__ to type this pattern.

@bryanforbes
Copy link
Contributor

I forgot to give an example of default and bound:

class Comparator(Protocol):
    def compare(self, other: Any) -> bool: ...

class Null:
    def compare(self, other: Any) -> bool: ...

class Integer:
    def compare(self, other: Any) -> bool: ...

T = TypeVar('T', bound=Comparator, default=Null)
class Clause(Generic[T]):
    type: T
    def __init__(self, , type: Optional[Type[T]] = None) -> None:
        if type is None:
            type = Null  # Something like this happens at runtime
        self.type = type()

reveal_type(Clause(Integer))  # OK, `Clause[Integer]`
reveal_type(Clause()) # Ok, `Clause[Null]`
reveal_type(Clause(int)) # Value of type variable "T" of "Clause" cannot be "int"
reveal_type(c = cast(Clause[Integer], {}))  # Ok, `Clause[Integer]`
reveal_type(c = cast(Clause, {}))  # Ok, `Clause[Null]`
reveal_type(c = cast(Clause[int], {})) # Type argument "builtins.int" of "Clause" must be a subtype of "Comparator"

@FuegoFro
Copy link
Contributor

FuegoFro commented Mar 7, 2019

For what it's worth, the workaround suggested in the first bullet of #4236 (comment), namely using overloads to handle the default argument, doesn't work if the function has a catch-all **kwargs: Any.

In particular, I'm looking into making the click.group decorator respect the optional cls keyword arg (so that mypy understands that the returned object is an instance of the cls type, rather than always a click.Group). Because this decorator needs to take **kwargs: Any to pass along to the cls constructor, any @overloads will always match the first overload. At least in this particular case, something like the proposed default option (or respecting the default arg) would be useful.

@emmatyping
Copy link
Collaborator

This came up again (if I'm not mistaken) in #6525. Raising priority to high.

@andersk
Copy link
Contributor

andersk commented Aug 14, 2019

I don’t think the T = TypeVar('T', default=str) concept makes sense as stated. Going back to @bryanforbes’ TypeScript vs. proposed Python examples:

  • in the TypeScript, the T = Null default means that T is Null unless T is provided, while
  • in the proposed Python, the TypeVar('T', default=Null) default purportedly means that T is Null unless type: Optional[Type[T]] is provided.

But on what grounds is mypy to reason that the type parameter is what controls T? What if there are multiple parameters referencing T? And how is mypy to notice the type error in Clause[int]() where T is provided but type is not?

On the other hand, if you could write

T = TypeVar('T')
class Clause(Generic[T]):
    type: T
    def __init__(self, type: Type[T] = Null) -> None:
        self.type = type()

then mypy ought to be able to infer that T is Null unless type is provided. I think all it needs to do is defer the type checking (or at least the type unification) of the default argument value from the declaration site to the call site. Related: #3737.

I’m coming here from a more complicated case in Zulip, where we have three type variables ItemT, CacheItemT, CompressedItemT and we want to infer that ItemT is CacheItemT unless an optional Callable[[ItemT], CacheItemT] parameter defaulting to an identity function is provided, and that CacheItemT is CompressedItemT unless a pair of optional Callable[[CacheItemT], CompressedItemT] and Callable[[CompressedItemT], CacheItemT] parameters defaulting to identity functions are provided.

@ilevkivskyi
Copy link
Member

Mypy already allows overloading __new__, so that one can already write:

T = TypeVar('T')
class Clause(Generic[T]):
    @overload
    def __new__(cls) -> Clause[Null]: ...
    @overload
    def __new__(cls, type: Type[T]) -> Clause[T]: ...

I think we can just add support for overloading on self in __init__ for those cases where actual implementation uses __init__ instead of __new__:

class Clause(Generic[T]):
    @overload
    def __init__(self: Clause[Null]) -> None: ...
    @overload
    def __init__(self: Clause[T], type: Type[T]) -> None: ...

@ilevkivskyi
Copy link
Member

Now that both overloading __new__() and overloading __init__() work correctly (see my comment just above), I think explicit type variable defaults is pretty low priority.

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 29, 2020

Type variable defaults should be discussed on typing-sig@ or the typing issue tracker, since they require changes to the typing module.

@vdwees
Copy link

vdwees commented May 27, 2024

To close the loop, PEP 696 – Type Defaults for Type Parameters was accepted and will land in Python 3.13.

edit: here is the updated issue number: #14851

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

9 participants