sdispater / pendulum

Python datetimes made easy
https://pendulum.eustace.io
MIT License
6.18k stars 381 forks source link

Incompatible type reported by mypy when using pendulum.parse #620

Open yoanisgil opened 2 years ago

yoanisgil commented 2 years ago

Issue

Given the following code:

import dataclasses
import pendulum
from pendulum import parse
from pendulum.datetime import DateTime

@dataclasses.dataclass
class A:
    creation_timestamp: DateTime

if __name__ == "__main__":
    a = A(creation_timestamp=pendulum.parse("2022-05-10T04:19:26Z"))
    print(a.creation_timestamp)

when I run type checking with mypy (version 0.950). I get the following error:

test_pendulum.py:11: error: Argument "creation_timestamp" to "A" has incompatible type "Union[Date, Time, Duration]"; expected "DateTime"

which is kind of surprising as one would say that the type hints here are correct:


def parse(
    text, **options
):  # type: (str, **typing.Any) -> typing.Union[Date, Time, DateTime, Duration]

Apparently mypy thinks pendulum.parse returns Union[Date, Time, Duration] (instead of Union[Date, Time, DateTime, Duration]). Any ideas why?

rileyjohngibbs commented 2 years ago

From the point of view of a type checker, a return value of Union[Date, Time, Duration] is equivalent to Union[Date, Time, DateTime, Duration] because DateTime is a subclass of Date.

That said, the typechecker is complaining because you are declaring that A.creation_timestamp must be DateTime object, but it could be getting Date, Time, or Duration. This is important because DateTime, being a subclass of Date, has attributes that Date (and Time and Duration) do not.

Simple example of why Union does this. Suppose you have these classes and this function that returns a union of them:

@dataclass
class Foo:
    foo: int
    common: int

@dataclass
class Bar:
    bar: int
    common: int

@dataclass
class SubFoo(Foo):
    subfoo: int

def get_something() -> Union[Foo, Bar]: ...

Now if you take the return value from get_something, it could be any of the three classes above, so if you try to use any attribute other than common, you could get an AttributeError. For example:

something: SubFoo = get_something()  # mypy will and should complain about this...
something.subfoo  # ...because this may raise an AttributeError

The only thing you should expect to be able to use on a return value from get_something is common:

something: Union[Foo, Bar] = get_something()
something.common  # no problem because defined on both Foo and Bar

Basically, you want a DateTime and you might want to use, say, creation_timestamp.set(minutes=5), which isn't valid for a Date object, because it's defined on DateTime. So mypy is warning you that you shouldn't expect to be able to use the attributes of DateTime.

kevinleahy-switchdin commented 3 months ago

Just to note, this is still an issue on 3.0.0.

Example:

import pendulum

start = pendulum.parse("2024-01-01 15:00")
end = pendulum.parse("2024-01-03 23:00")

def bad_func(start: pendulum.DateTime, end: pendulum.DateTime):
    pass

bad_func(start, end)

Pyright gives the error

Argument of type "Date | Time | DateTime | Duration" cannot be assigned to parameter "start" of type "DateTime" in function "bad_func"
  Type "Date | Time | DateTime | Duration" is incompatible with type "DateTime"
    "Date" is incompatible with "DateTime"Pylance[reportArgumentType](https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportArgumentType)
rileyjohngibbs commented 3 months ago

Just to note, this is still an issue on 3.0.0.

I think you're identifying a slightly different thing.

In either case, the behavior is correct. As the error message from Pyright says, because start is possibly a Date instance (and not necessarily a DateTime instance), it cannot be safely passed to bad_func, which expects an instance specifically of DateTime.

I'd recommend, where appropriate, using something like:

assert isinstance(start, pendulum.DateTime)
assert isinstance(end, pendulum.DateTime)
kevinleahy-switchdin commented 3 months ago

Ah, @rileyjohngibbs you're correct! Should this issue be closed, as it's expected and correct behaviour?

rileyjohngibbs commented 3 months ago

I think it should be closed, @kevinleahy-switchdin, yes. And you're welcome! Glad I could help.