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

datetime.datetime is not Liskov-substitutable for datetime.date, but it's typed as if it were #3805

Closed
glyph opened this issue Mar 3, 2020 · 6 comments

Comments

@glyph
Copy link
Contributor

glyph commented Mar 3, 2020

Consider this code:

import datetime

def compare_dates(x: datetime.date, y: datetime.date) -> bool:
    return x > y

compare_dates(datetime.datetime.now(), datetime.date.today())

Under mypy, it typechecks. Mypy's job is to prevent runtime TypeErrors, right? At runtime:

Traceback (most recent call last):
  File "<input>", line 1, in <module>
    compare_dates(datetime.datetime.now(), datetime.date.today())
  File "<input>", line 2, in compare_dates
    return x > y
TypeError: can't compare datetime.datetime to datetime.date

I think #2487 was wrong, and it actually makes more sense to type datetime.date and datetime.datetime separately.

Perhaps this is a mypy bug though? Maybe just because something's a subclass, doesn't mean it should be a subtype? Or a bug in datetime because it shouldn't do this?

I'm not sure. But practically speaking this makes any code working with date objects in mypy somewhat misleading and fraught.

@glyph
Copy link
Contributor Author

glyph commented Mar 3, 2020

(It seems like mypy is aware of the substitutability problem here, and they've all been type:ignore'd away?

def __le__(self, other: datetime) -> bool: ... # type: ignore
def __lt__(self, other: datetime) -> bool: ... # type: ignore
def __ge__(self, other: datetime) -> bool: ... # type: ignore
def __gt__(self, other: datetime) -> bool: ... # type: ignore
)

@srittau
Copy link
Collaborator

srittau commented Mar 4, 2020

datetime is a subclass of date and can generally be used in place of it. Not reflecting this relationship in the stubs would cause countless other problems. I don't think there is a good way to represent that datetime violates the LSP in typeshed. Please note that that is true for many classes as well. Nevertheless it has been my experience that generally it's better for stubs to follow the implementation as closely as possibly, but we can't guard against all peculiarities of implementations.

@glyph
Copy link
Contributor Author

glyph commented Mar 11, 2020

@srittau the presence of those four type: ignores suggests to me that there's something missing from the stubs, though; this isn't just a matter of the stubs accurately representing the code and losing some information; mypy is telling us that this is a broken thing to do, the information is being thrown away.

@srittau
Copy link
Collaborator

srittau commented Mar 11, 2020

There is not much typeshed can do here. The implementation does not adhere to the LSP, the stubs follow that implementation.

@srittau srittau closed this as completed Mar 11, 2020
@glyph
Copy link
Contributor Author

glyph commented Jun 17, 2020

@srittau This is still a bug - where should it be filed? Mypy? Core python?

@srittau
Copy link
Collaborator

srittau commented Jun 17, 2020

Python core decided to violate the LSP with this class hierarchy and mypy decided to warn about LSP violations. Either of those places, although I am sure that the mypy developers see the warning as a feature.

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

No branches or pull requests

2 participants