From b5080e8f27cdcba2d8fcebeca9a3257199a78239 Mon Sep 17 00:00:00 2001 From: Matthew Harrigan Date: Tue, 31 May 2022 10:50:54 -0700 Subject: [PATCH 1/5] Datetime policy --- cirq-core/cirq/json_resolver_cache.py | 11 +++++------ cirq-core/cirq/protocols/json_serialization.py | 7 ------- docs/dev/style.md | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/cirq-core/cirq/json_resolver_cache.py b/cirq-core/cirq/json_resolver_cache.py index c0ca882caff..d34f338a8ee 100644 --- a/cirq-core/cirq/json_resolver_cache.py +++ b/cirq-core/cirq/json_resolver_cache.py @@ -55,13 +55,12 @@ def _parallel_gate_op(gate, qubits): return cirq.parallel_gate_op(gate, *qubits) def _datetime(timestamp: float) -> datetime.datetime: - # As part of our serialization logic, we make sure we only serialize "aware" - # datetimes with the UTC timezone, so we implicitly add back in the UTC timezone here. + # We serialize datetimes (both with ("aware") and without ("naive") timezone information) + # as unix timestamps. The deserialized datetime will always refer to the + # same point in time, but will be re-constructed as a timezone-aware object. # - # Please note: even if the assumption is somehow violated, the fact that we use - # unix timestamps should mean that the deserialized datetime should refer to the - # same point in time but may not satisfy o = read_json(to_json(o)) because the actual - # timezones, and hour fields will not be identical. + # If `o` is a naive datetime, o != read_json(to_json(o)) because Python doesn't + # let you compare aware and naive datetimes. return datetime.datetime.fromtimestamp(timestamp, tz=datetime.timezone.utc) def _symmetricalqidpair(qids): diff --git a/cirq-core/cirq/protocols/json_serialization.py b/cirq-core/cirq/protocols/json_serialization.py index d66ed3f6615..757037edf24 100644 --- a/cirq-core/cirq/protocols/json_serialization.py +++ b/cirq-core/cirq/protocols/json_serialization.py @@ -398,13 +398,6 @@ def default(self, o): # datetime if isinstance(o, datetime.datetime): - if o.tzinfo is None or o.tzinfo.utcoffset(o) is None: - # Otherwise, the deserialized object may change depending on local timezone. - raise TypeError( - "Can only serialize 'aware' datetime objects with `tzinfo`. " - "Consider using e.g. `datetime.datetime.now(tz=datetime.timezone.utc)`" - ) - return {'cirq_type': 'datetime.datetime', 'timestamp': o.timestamp()} return super().default(o) # coverage: ignore diff --git a/docs/dev/style.md b/docs/dev/style.md index dab87136020..a5f30841636 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -93,3 +93,18 @@ later in the same file. Using consistent wording across Cirq is important for lowering users cognitive load. For rule governing naming, see the [nomenclature guidelines](nomenclature.md). + +## Datetimes + +Prefer using timezone-aware `datetime` objects. + +```python +import datetime +dt = datetime.datetime.now(tz=datetime.timezone.utc) +``` + +Protobuf APIs will return "aware" `datetime` objects. JSON de-serialization will +promote values to "aware" `datetime` objects upon deserialization. + +Absolutely do not use `datetime.utcnow()` as explained in the warnings in the +Python [documentation](https://docs.python.org/3/library/datetime.html). \ No newline at end of file From 1f7c72b3dc6fca5523ff21ef2d437d1059211566 Mon Sep 17 00:00:00 2001 From: Matthew Harrigan Date: Tue, 31 May 2022 11:00:00 -0700 Subject: [PATCH 2/5] tests --- cirq-core/cirq/protocols/json_serialization_test.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/cirq-core/cirq/protocols/json_serialization_test.py b/cirq-core/cirq/protocols/json_serialization_test.py index e969e7f0f5f..dbc17ab6bed 100644 --- a/cirq-core/cirq/protocols/json_serialization_test.py +++ b/cirq-core/cirq/protocols/json_serialization_test.py @@ -951,11 +951,16 @@ def test_basic_time_assertions(): def test_datetime(): naive_dt = datetime.datetime.now() - with pytest.raises(TypeError): - cirq.to_json(naive_dt) + re_naive_dt = cirq.read_json(json_text=cirq.to_json(naive_dt)) + assert re_naive_dt != naive_dt, 'loads in with timezone' utc_dt = naive_dt.astimezone(datetime.timezone.utc) - assert utc_dt == cirq.read_json(json_text=cirq.to_json(utc_dt)) + re_utc_dt = cirq.read_json(json_text=cirq.to_json(utc_dt)) + assert re_utc_dt == utc_dt + assert re_utc_dt == re_naive_dt pst_dt = naive_dt.astimezone(tz=datetime.timezone(offset=datetime.timedelta(hours=-8))) - assert utc_dt == cirq.read_json(json_text=cirq.to_json(pst_dt)) + re_pst_dt = cirq.read_json(json_text=cirq.to_json(pst_dt)) + assert re_pst_dt == pst_dt + assert re_pst_dt == utc_dt + assert re_pst_dt == re_naive_dt From c1ef28e31ec9fe3a97980707f140befed0c76d64 Mon Sep 17 00:00:00 2001 From: Matthew Harrigan Date: Wed, 8 Jun 2022 15:11:43 -0700 Subject: [PATCH 3/5] dabacon wording --- docs/dev/style.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/dev/style.md b/docs/dev/style.md index a5f30841636..048d64b6021 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -103,8 +103,8 @@ import datetime dt = datetime.datetime.now(tz=datetime.timezone.utc) ``` -Protobuf APIs will return "aware" `datetime` objects. JSON de-serialization will -promote values to "aware" `datetime` objects upon deserialization. +Public components of Protobuf APIs will return "aware" `datetime` objects. +JSON de-serialization will promote values to "aware" `datetime` objects upon deserialization. Absolutely do not use `datetime.utcnow()` as explained in the warnings in the Python [documentation](https://docs.python.org/3/library/datetime.html). \ No newline at end of file From 62efb496b317f5fae1800ed858b75b35087ab159 Mon Sep 17 00:00:00 2001 From: Matthew Harrigan Date: Wed, 8 Jun 2022 15:12:32 -0700 Subject: [PATCH 4/5] timestamp test --- cirq-core/cirq/protocols/json_serialization_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cirq-core/cirq/protocols/json_serialization_test.py b/cirq-core/cirq/protocols/json_serialization_test.py index dbc17ab6bed..69d0e3ae4a6 100644 --- a/cirq-core/cirq/protocols/json_serialization_test.py +++ b/cirq-core/cirq/protocols/json_serialization_test.py @@ -953,6 +953,7 @@ def test_datetime(): re_naive_dt = cirq.read_json(json_text=cirq.to_json(naive_dt)) assert re_naive_dt != naive_dt, 'loads in with timezone' + assert re_naive_dt.timestamp() == naive_dt.timestamp() utc_dt = naive_dt.astimezone(datetime.timezone.utc) re_utc_dt = cirq.read_json(json_text=cirq.to_json(utc_dt)) From 91b5c5188999b9f0bbb63882c90ef49d847e788a Mon Sep 17 00:00:00 2001 From: Matthew Harrigan Date: Wed, 8 Jun 2022 15:22:38 -0700 Subject: [PATCH 5/5] comparison clarification --- docs/dev/style.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/dev/style.md b/docs/dev/style.md index 048d64b6021..5bedee38e97 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -106,5 +106,13 @@ dt = datetime.datetime.now(tz=datetime.timezone.utc) Public components of Protobuf APIs will return "aware" `datetime` objects. JSON de-serialization will promote values to "aware" `datetime` objects upon deserialization. +Comparing (or testing equality) between "naive" and "aware" `datetime` objects throws +an exception. +If you are implementing a class that has `datetime` member variables, delegate equality +and comparison operators to the built-in `datetime` equality and comparison operators. +If you're writing a function that compares `datetime` objects, you can defensively promote +them to "aware" objects or use their `.timestamp()` properties for a comparison that will +never throw an exception. + Absolutely do not use `datetime.utcnow()` as explained in the warnings in the Python [documentation](https://docs.python.org/3/library/datetime.html). \ No newline at end of file