Skip to content

BUG: Fix Timestamp('now') and Timestamp.now unit inconsistency #56281

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Dec 7, 2023
Merged

BUG: Fix Timestamp('now') and Timestamp.now unit inconsistency #56281

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d6fdbee
BUG: Fix `Timestamp('now')` and `Timestamp.now` unit inconsistency
yuanx749 Dec 1, 2023
ec846d2
Merge remote-tracking branch 'upstream/main' into timestamp_now_unit
yuanx749 Dec 2, 2023
1af8ab0
Move return
yuanx749 Dec 2, 2023
c24887e
Update doc/source/whatsnew/v2.1.4.rst
yuanx749 Dec 5, 2023
cc4be07
Merge remote-tracking branch 'upstream/main' into timestamp_now_unit
yuanx749 Dec 5, 2023
f34f18f
Resolve
yuanx749 Dec 5, 2023
25778eb
not use tuple in cython for perf
yuanx749 Dec 7, 2023
f35d87c
Merge remote-tracking branch 'upstream/main' into timestamp_now_unit
yuanx749 Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.1.4.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Fixed regressions
Bug fixes
~~~~~~~~~
- Bug in :class:`Series` constructor raising DeprecationWarning when ``index`` is a list of :class:`Series` (:issue:`55228`)
- Bug in :class:`Timestamp` construction with ``ts_input=now`` or ``ts_input=today`` giving a different unit from :meth:`Timestamp.now` or :meth:`Timestamp.today` (:issue:`55879`)
- Bug in :meth:`Index.__getitem__` returning wrong result for Arrow dtypes and negative stepsize (:issue:`55832`)
- Fixed bug in :func:`to_numeric` converting to extension dtype for ``string[pyarrow_numpy]`` dtype (:issue:`56179`)
- Fixed bug in :meth:`DataFrame.__setitem__` casting :class:`Index` with object-dtype to PyArrow backed strings when ``infer_string`` option is set (:issue:`55638`)
Expand Down
2 changes: 1 addition & 1 deletion pandas/_libs/tslibs/conversion.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ cdef _TSObject convert_str_to_tsobject(str ts, tzinfo tz,
reso = get_supported_reso(out_bestunit)
return convert_datetime_to_tsobject(dt, tz, nanos=nanos, reso=reso)

return convert_datetime_to_tsobject(dt, tz)
return convert_datetime_to_tsobject(dt, tz, nanos=0, reso=NPY_FR_us)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This general case probably cannot be hardcoded

Copy link
Contributor Author

@yuanx749 yuanx749 Dec 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the only cases left to handle here are 'now' and 'today', all other cases have returned.

I think we could combine these two cases and then return, to make the code more clear.

pandas/pandas/_libs/tslibs/conversion.pyx

Lines 597 to 605 in d6fdbee

elif ts == "now":
# Issue 9000, we short-circuit rather than going
# into np_datetime_strings which returns utc
dt = datetime.now(tz)
elif ts == "today":
# Issue 9000, we short-circuit rather than going
# into np_datetime_strings which returns a normalized datetime
dt = datetime.now(tz)
# equiv: datetime.today().replace(tzinfo=tz)



cdef check_overflows(_TSObject obj, NPY_DATETIMEUNIT reso=NPY_FR_ns):
Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/scalar/timestamp/test_constructors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,20 @@ def test_constructor_str_infer_reso(self):
ts = Timestamp("2020-01-01 00+00:00")
assert ts.unit == "s"

def test_now_today_unit(self):
# GH#55879
ts_now_from_method = Timestamp.now()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use pytest.mark.parametrize?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

ts_now_from_string = Timestamp("now")
ts_today_from_method = Timestamp.today()
ts_today_from_string = Timestamp("today")
assert (
ts_now_from_method.unit
== ts_now_from_string.unit
== ts_today_from_method.unit
== ts_today_from_string.unit
== "us"
)


class TestTimestampConstructors:
def test_weekday_but_no_day_raises(self):
Expand Down
4 changes: 2 additions & 2 deletions pandas/tests/tools/test_to_datetime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ def test_to_datetime_now(self):
# See GH#18666
with tm.set_timezone("US/Eastern"):
# GH#18705
now = Timestamp("now")
now = Timestamp("now").as_unit("ns")
pdnow = to_datetime("now")
pdnow2 = to_datetime(["now"])[0]

Expand All @@ -1066,7 +1066,7 @@ def test_to_datetime_today(self, tz):
pdtoday = to_datetime("today")
pdtoday2 = to_datetime(["today"])[0]

tstoday = Timestamp("today")
tstoday = Timestamp("today").as_unit("ns")
tstoday2 = Timestamp.today().as_unit("ns")

# These should all be equal with infinite perf; this gives
Expand Down