BUG: Support timespec argument in Timestamp.isoformat() (#26131) #38550
Conversation
| @@ -607,6 +607,7 @@ Datetimelike | |||
| - Bug in :meth:`Series.isin` with ``datetime64[ns]`` dtype and :meth:`.DatetimeIndex.isin` failing to consider timezone-aware and timezone-naive datetimes as always different (:issue:`35728`) | |||
| - Bug in :meth:`Series.isin` with ``PeriodDtype`` dtype and :meth:`PeriodIndex.isin` failing to consider arguments with different ``PeriodDtype`` as always different (:issue:`37528`) | |||
| - Bug in :class:`Period` constructor now correctly handles nanoseconds in the ``value`` argument (:issue:`34621` and :issue:`17053`) | |||
| - Bug in :meth:`Timestamp.isoformat`, now handles the ``timespec`` argument from the base :class:``datetime`` class (:issue:`26131`) | |||
There was a problem hiding this comment.
The issue is tagged as enhancement so this probably should be moved there
Also I would expect this to target v1.3 not v1.2
There was a problem hiding this comment.
yeah pls move this to 1.3 under other enhancements
|
|
||
|
|
||
| def test_isoformat(): | ||
| ts = Timestamp( |
There was a problem hiding this comment.
Does this handle a nanosecond argument?
| @@ -607,6 +607,7 @@ Datetimelike | |||
| - Bug in :meth:`Series.isin` with ``datetime64[ns]`` dtype and :meth:`.DatetimeIndex.isin` failing to consider timezone-aware and timezone-naive datetimes as always different (:issue:`35728`) | |||
| - Bug in :meth:`Series.isin` with ``PeriodDtype`` dtype and :meth:`PeriodIndex.isin` failing to consider arguments with different ``PeriodDtype`` as always different (:issue:`37528`) | |||
| - Bug in :class:`Period` constructor now correctly handles nanoseconds in the ``value`` argument (:issue:`34621` and :issue:`17053`) | |||
| - Bug in :meth:`Timestamp.isoformat`, now handles the ``timespec`` argument from the base :class:``datetime`` class (:issue:`26131`) | |||
There was a problem hiding this comment.
yeah pls move this to 1.3 under other enhancements
| def isoformat(self, sep: str = "T") -> str: | ||
| base = super(_Timestamp, self).isoformat(sep=sep) | ||
| def isoformat(self, sep: str = "T", timespec: str = "auto") -> str: | ||
| base = super(_Timestamp, self).isoformat(sep=sep, timespec=timespec) |
There was a problem hiding this comment.
can you also add a doc-string here
| @@ -609,8 +609,8 @@ cdef class _Timestamp(ABCTimestamp): | |||
| # ----------------------------------------------------------------- | |||
| # Rendering Methods | |||
|
|
|||
| def isoformat(self, sep: str = "T") -> str: | |||
| base = super(_Timestamp, self).isoformat(sep=sep) | |||
| def isoformat(self, sep: str = "T", timespec: str = "auto") -> str: | |||
There was a problem hiding this comment.
is 'auto' the typical name for this?
| from pandas import Timestamp | ||
|
|
||
|
|
||
| def test_isoformat(): |
There was a problem hiding this comment.
can you locate with with other formatting tests
There was a problem hiding this comment.
Can you suggest a more specific location that would be better? I looked, but couldn't find an obvious place where other formatting tests are located.
|
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
|
Thanks for the PR, but going to close due the lack of activity. Let us know if you'd like to continue working on this. |
|
I'm planning to take over this PR as the original author seems to have lost time/interest. |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffI have just tried to support the same behaviour as
datetime.datetime(), without adding the additional"nanoseconds"option, but this could be handled as well.