-
Notifications
You must be signed in to change notification settings - Fork 288
-
Hi All,
Is this a bug in mypy? As best I (and a bunch of unit tests!) can tell, this code is good:
from datetime import datetime def add( cls: type[datetime], instance: datetime | int | None = None, /, *args: int, **kw: int, ) -> datetime: if isinstance(instance, datetime): pass elif isinstance(instance, int): instance = cls(instance, *args, **kw) else: instance = cls(*args, **kw) return instance
...but mypy (1.15.0 runnong on Python 3.12.1) complains with:
$ mypy reproducer.py
reproducer.py:14: error: Argument 2 to "datetime" has incompatible type "*tuple[int, ...]"; expected "tzinfo | None" [arg-type]
reproducer.py:14: error: Argument 3 to "datetime" has incompatible type "**dict[str, int]"; expected "tzinfo | None" [arg-type]
reproducer.py:16: error: Argument 1 to "datetime" has incompatible type "*tuple[int, ...]"; expected "tzinfo | None" [arg-type]
reproducer.py:16: error: Argument 2 to "datetime" has incompatible type "**dict[str, int]"; expected "tzinfo | None" [arg-type]
Found 4 errors in 1 file (checked 1 source file)
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 2 comments 2 replies
-
Mypy is accurately telling you that the datetime constructor has an argument (tzinfo) which accepts either a tzinfo instance or None, and you can't pass an int to that argument, but your code might.
I would say it feels slightly arbitrary what the type checker chooses to complain about here, in the sense that this code is much more broadly type-unsafe than that: any invalid keyword name provided to this wrapper, or too many positional arguments, will also result in a TypeError from the datetime constructor call, and the type checker provides no help in preventing that. But type checkers choose to assume that you are you are taking it upon yourself to ensure only the correct argument names (and/or number of positional arguments) are passed when you use *args and **kwargs like this, but it does still take on the job of alerting you that you might be passing the wrong argument type for some arguments.
This might be a good use case for ParamSpec? You might be able to actually teach the type checker to know what are the valid arguments to provide to add.
Beta Was this translation helpful? Give feedback.
All reactions
-
Mypy is accurately telling you that the datetime constructor has an argument (tzinfo) which accepts either a tzinfo instance or None, and you can't pass an int to that argument, but your code might.
I'd argue about accurately, or certainly "helpfully", as this specific error message didn't communicate to what your text cleared up:
reproducer.py:14: error: Argument 2 to "datetime" has incompatible type "*tuple[int, ...]"; expected "tzinfo | None" [arg-type]
This would have been easier for me to understanding:
reproducer.py:14: error: Argument 2 to "datetime" has incompatible type "*tuple[int, ...]"; expected "*tuple[int | tzinfo | None, ...]" [arg-type]
I guess we're also running into limitations of *args typing: there's no way to say "this may only be up to this length". What's annoying is that even with added overload definitions, mypy still doesn't infer that this problem can't be hit:
from datetime import datetime, tzinfo from typing import overload @overload def add( cls: type[datetime], year: int, month: int, day: int, hour: int = 0, minute: int = 0, second: int = 0, microsecond: int = 0, tzinfo: tzinfo | None = None, ) -> datetime: ... @overload def add( cls: type[datetime], instance: datetime, /, ) -> datetime: ... def add( cls: type[datetime], instance: datetime | int | None = None, /, *args: int, tzinfo: tzinfo | None = None, **kw: int, ) -> datetime: if isinstance(instance, datetime): pass elif isinstance(instance, int): instance = cls(instance, *args, tzinfo=tzinfo, **kw) else: instance = cls(*args, tzinfo=tzinfo, **kw) return instance
...in fact, more errors pop up :-(
$ mypy reproducer.py
reproducer.py:29: error: Overloaded function implementation does not accept all possible arguments of signature 1 [misc]
reproducer.py:40: error: "datetime" gets multiple values for keyword argument "tzinfo" [misc]
reproducer.py:40: error: Argument 2 to "datetime" has incompatible type "*tuple[int, ...]"; expected "tzinfo | None" [arg-type]
reproducer.py:40: error: Argument 4 to "datetime" has incompatible type "**dict[str, int]"; expected "tzinfo | None" [arg-type]
reproducer.py:42: error: "datetime" gets multiple values for keyword argument "tzinfo" [misc]
reproducer.py:42: error: Argument 1 to "datetime" has incompatible type "*tuple[int, ...]"; expected "tzinfo | None" [arg-type]
reproducer.py:42: error: Argument 3 to "datetime" has incompatible type "**dict[str, int]"; expected "tzinfo | None" [arg-type]
Found 7 errors in 1 file (checked 1 source file)
If you could show me how to use ParamSpec in this situation, I'd be very grateful!
Beta Was this translation helpful? Give feedback.
All reactions
-
Quibbles with mypy error message wording should go to the mypy bug tracker, not here.
This is the closest I can get with ParamSpec on a quick try:
from datetime import datetime from typing import Callable def add[**P]( cls: Callable[P, datetime], instance: datetime | int | None = None, /, *args: P.args, **kw: P.kwargs, ) -> datetime: if isinstance(instance, datetime): pass elif isinstance(instance, int): instance = cls(instance, *args, **kw) else: instance = cls(*args, **kw) return instance
It requires changing the cls argument to be any callable that returns a datetime, rather than type[datetime] specifically. Personally I think this is entirely an improvement: it makes the function more flexible for callers, and it's better because callability of type is unsafe to begin with (because Liskov is not enforced for __init__ or __new__), so using a Callable annotation instead of a type annotation will give correct behavior, instead of just silently being unsound, if someone passes a subclass of datetime with a different constructor signature.
But it can't handle the call with prepended instance argument. This call can be made OK by using Callable[typing.Concatenate[int, P], datetime] instead, but then the second call isn't happy. I don't see a way to make that level of dynamism work with the available typing features. I think your best bet might be to use overloads with ParamSpec to achieve really good external typing for the function, but you're not going to get good typing of the internal function body that way: typing of the function body doesn't understand its external overloads at all. This is probably what I'd go with for this case:
from datetime import datetime from typing import Callable, Concatenate, overload @overload def add[**P]( cls: Callable[P, datetime], instance: datetime | None = None, /, *args: P.args, **kwargs: P.kwargs, ) -> datetime: ... @overload def add[**P]( cls: Callable[Concatenate[int, P], datetime], instance: int, /, *args: P.args, **kwargs: P.kwargs, ) -> datetime: ... def add( cls: Callable[..., datetime], instance: datetime | int | None = None, /, *args, **kw, ) -> datetime: if isinstance(instance, datetime): pass elif isinstance(instance, int): instance = cls(instance, *args, **kw) else: instance = cls(*args, **kw) return instance
Beta Was this translation helpful? Give feedback.
All reactions
-
See #2040 for newer discussion on this.
Beta Was this translation helpful? Give feedback.