Skip to content

BUG: to_datetime can return NaTType #88

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
bashtage opened this issue Jul 4, 2022 · 3 comments · Fixed by #115
Closed

BUG: to_datetime can return NaTType #88

bashtage opened this issue Jul 4, 2022 · 3 comments · Fixed by #115

Comments

@bashtage
Copy link
Contributor

bashtage commented Jul 4, 2022

import pandas as pd
out = pd.to_datetime("dajsldaljd", errors="coerce")
reveal_type(out)

wrong type returns

note: Revealed type is "pandas._libs.tslibs.timestamps.Timestamp"

Also, mypy on

import pandas as pd
out = pd.to_datetime("dajsldaljd", errors="coerce")
out is pd.NaT

returns

error: Non-overlapping identity check (left operand type: "Timestamp", right operand type: "NaTType")
@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Jul 5, 2022

We made a conscious decision to not have pd.to_datetime() return Union[pd.NaT, Timestamp] because then you'd have to use cast and write this in lots of code:

out = cast(pd.Timestamp, pd.to_datetime("2022-07-05"))

to force out to be pd.Timestamp, which is what you want most of the time.

There's not a way in static typing to test if the timestamp is valid.

@Dr-Irv Dr-Irv closed this as completed Jul 5, 2022
@bashtage
Copy link
Contributor Author

bashtage commented Jul 5, 2022

@Dr-Irv why not @overload with errors:Literal["coerce"] with a return type [Timestamp, NaTType] and errors:Literal["ignore","raise"] with return type Timestamp? This type of overloading would let most users ignore the NaT and the cast while letting those who use errors="coerce" get correct typing.

Reading python/mypy#5090 both convinced me that str and Sequence[str] are the same, and that the role of a typechecker is to be as honest as possible with the underlying language, including undesirable warts.

@Dr-Irv Dr-Irv reopened this Jul 6, 2022
@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Jul 6, 2022

@Dr-Irv why not @overload with errors:Literal["coerce"] with a return type [Timestamp, NaTType] and errors:Literal["ignore","raise"] with return type Timestamp? This type of overloading would let most users ignore the NaT and the cast while letting those who use errors="coerce" get correct typing.

Reasonable suggestion. Open to a PR. Have reopened the issue.

Reading python/mypy#5090 both convinced me that str and Sequence[str] are the same, and that the role of a typechecker is to be as honest as possible with the underlying language, including undesirable warts.

Not sure that is relevant here - I think you're referring to #82, so comment there on your perspective on this issue.

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

Successfully merging a pull request may close this issue.

2 participants