-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
API: allow mixed-datetimes-and-ints in to_datetime, DatetimeIndex #49348
Conversation
to_datetime(arr, errors="raise", cache=cache) | ||
# GH#49037 pre-2.0 this raised, but it always worked with Series, | ||
# was never clear why it was disallowed | ||
result = to_datetime(arr, errors="raise", cache=cache) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a test where unit
and/or origin
is also specified in this mixed cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont think so, no
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this test be easily added? ( I would expect unit
and origin
only applicable to the ints and ignored for datetimes )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
origin is only relevant with numeric dtypes, so won't go through array_to_datetime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally +1 to have [ints, datetimes]
be supported, especially in to_datetime
I commented on the issue (#49037), I think I am also +1 on allowing the mixture (although I don't like that specific behaviour, it seems the most consistent if we want to have generally applicable rules) @jbrockmendel one thing I am wondering: the |
Assuming you mean in array_to_datetime. I don't think its particularly useful, but its also not harmful (except for not being tested). For only-datetime-objects (i.e. the strict from_scalars) i think it may be cleaner to implement a new dedicated function than further overloading array_to_datetime. |
failures unrelated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @jbrockmendel |
…ndas-dev#49348) * API: allow mixed-datetimes-and-ints in to_datetime, DatetimeIndex * typo fixup * typo fixup, update import * mypy fixup
…ndas-dev#49348) * API: allow mixed-datetimes-and-ints in to_datetime, DatetimeIndex * typo fixup * typo fixup, update import * mypy fixup
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.cc @jreback this goes a long way towards getting Series/Index constructors to match, but i dont see the reason for the current behavior, do you remember?