From 4cca28001c01d3fa2d3ebdd940c7aa9ea044e107 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Thu, 27 Aug 2020 00:25:46 +0200 Subject: [PATCH 1/3] Fix bug roundtripping datetime.time objects after midnight in eastern hemisphere timezones (#2417) --- include/pybind11/chrono.h | 19 ++++++++++++++----- tests/test_chrono.cpp | 1 + tests/test_chrono.py | 15 ++++++++++++--- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/include/pybind11/chrono.h b/include/pybind11/chrono.h index 6b9ab9b82a..77d45f5fb9 100644 --- a/include/pybind11/chrono.h +++ b/include/pybind11/chrono.h @@ -150,21 +150,30 @@ template class type_caster(src)); + // Declare these special duration types so the conversions happen with the correct primitive types (int) + using us_t = duration; + + // Get out microseconds, and make sure they are positive, to avoid bug in eastern hemisphere time zones + // (cfr. https://github.com/pybind/pybind11/issues/2417) + auto us = duration_cast(src.time_since_epoch() % seconds(1)); + if (us.count() < 0) + us += seconds(1); + + // Subtract microseconds BEFORE `system_clock::to_time_t`, because: + // > If std::time_t has lower precision, it is implementation-defined whether the value is rounded or truncated. + // (https://en.cppreference.com/w/cpp/chrono/system_clock/to_time_t) + std::time_t tt = system_clock::to_time_t(time_point_cast(src - us)); // this function uses static memory so it's best to copy it out asap just in case // otherwise other code that is using localtime may break this (not just python code) std::tm localtime = *std::localtime(&tt); - // Declare these special duration types so the conversions happen with the correct primitive types (int) - using us_t = duration; - return PyDateTime_FromDateAndTime(localtime.tm_year + 1900, localtime.tm_mon + 1, localtime.tm_mday, localtime.tm_hour, localtime.tm_min, localtime.tm_sec, - (duration_cast(src.time_since_epoch() % seconds(1))).count()); + us.count()); } PYBIND11_TYPE_CASTER(type, _("datetime.datetime")); }; diff --git a/tests/test_chrono.cpp b/tests/test_chrono.cpp index 899d08d8d8..1d79d4b6ca 100644 --- a/tests/test_chrono.cpp +++ b/tests/test_chrono.cpp @@ -10,6 +10,7 @@ #include "pybind11_tests.h" #include +#include TEST_SUBMODULE(chrono, m) { using system_time = std::chrono::system_clock::time_point; diff --git a/tests/test_chrono.py b/tests/test_chrono.py index 5392f8ff0d..c9853fe13c 100644 --- a/tests/test_chrono.py +++ b/tests/test_chrono.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from pybind11_tests import chrono as m import datetime +import pytest def test_chrono_system_clock(): @@ -70,9 +71,17 @@ def test_chrono_system_clock_roundtrip_date(): assert time2.microsecond == 0 -def test_chrono_system_clock_roundtrip_time(): - time1 = datetime.datetime.today().time() - +@pytest.mark.parametrize("time1", [ + datetime.datetime.today().time(), + datetime.time(0, 0, 0), + datetime.time(0, 0, 0, 1), + datetime.time(0, 28, 45, 109827), + datetime.time(0, 59, 59, 999999), + datetime.time(1, 0, 0), + datetime.time(5, 59, 59, 0), + datetime.time(5, 59, 59, 1), +]) +def test_chrono_system_clock_roundtrip_time(time1): # Roundtrip the time datetime2 = m.test_chrono2(time1) date2 = datetime2.date() From 8a301ec20b199e0d8c672090c161ed9974f4a296 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 27 Aug 2020 09:52:52 -0400 Subject: [PATCH 2/3] tests: check more timezones --- tests/test_chrono.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_chrono.py b/tests/test_chrono.py index c9853fe13c..793226525d 100644 --- a/tests/test_chrono.py +++ b/tests/test_chrono.py @@ -81,7 +81,14 @@ def test_chrono_system_clock_roundtrip_date(): datetime.time(5, 59, 59, 0), datetime.time(5, 59, 59, 1), ]) -def test_chrono_system_clock_roundtrip_time(time1): +@pytest.mark.parametrize("tz", [ + "Europe/Brussels", + "Asia/Pyongyang", + "America/New_York", +]) +def test_chrono_system_clock_roundtrip_time(time1, tz, monkeypatch): + monkeypatch.setenv("TZ", "/usr/share/zoneinfo/{}".format(tz)) + # Roundtrip the time datetime2 = m.test_chrono2(time1) date2 = datetime2.date() From 83379f98ecc9a67d9f509be67908a1efcc387280 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Thu, 27 Aug 2020 23:59:43 +0200 Subject: [PATCH 3/3] Fix review remarks: remove useless comment and skip setting TZ environment variable on Windows --- include/pybind11/chrono.h | 4 +--- tests/test_chrono.py | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/include/pybind11/chrono.h b/include/pybind11/chrono.h index 77d45f5fb9..6127c659bd 100644 --- a/include/pybind11/chrono.h +++ b/include/pybind11/chrono.h @@ -150,11 +150,9 @@ template class type_caster; - // Get out microseconds, and make sure they are positive, to avoid bug in eastern hemisphere time zones // (cfr. https://github.com/pybind/pybind11/issues/2417) + using us_t = duration; auto us = duration_cast(src.time_since_epoch() % seconds(1)); if (us.count() < 0) us += seconds(1); diff --git a/tests/test_chrono.py b/tests/test_chrono.py index 793226525d..f94d5ba979 100644 --- a/tests/test_chrono.py +++ b/tests/test_chrono.py @@ -3,6 +3,8 @@ import datetime import pytest +import env # noqa: F401 + def test_chrono_system_clock(): @@ -71,6 +73,11 @@ def test_chrono_system_clock_roundtrip_date(): assert time2.microsecond == 0 +SKIP_TZ_ENV_ON_WIN = pytest.mark.skipif( + "env.WIN", reason="TZ environment variable only supported on POSIX" +) + + @pytest.mark.parametrize("time1", [ datetime.datetime.today().time(), datetime.time(0, 0, 0), @@ -82,12 +89,14 @@ def test_chrono_system_clock_roundtrip_date(): datetime.time(5, 59, 59, 1), ]) @pytest.mark.parametrize("tz", [ - "Europe/Brussels", - "Asia/Pyongyang", - "America/New_York", + None, + pytest.param("Europe/Brussels", marks=SKIP_TZ_ENV_ON_WIN), + pytest.param("Asia/Pyongyang", marks=SKIP_TZ_ENV_ON_WIN), + pytest.param("America/New_York", marks=SKIP_TZ_ENV_ON_WIN), ]) def test_chrono_system_clock_roundtrip_time(time1, tz, monkeypatch): - monkeypatch.setenv("TZ", "/usr/share/zoneinfo/{}".format(tz)) + if tz is not None: + monkeypatch.setenv("TZ", "/usr/share/zoneinfo/{}".format(tz)) # Roundtrip the time datetime2 = m.test_chrono2(time1)