From 07c7b4f151022f8ecdbaff6013cc71e36340d155 Mon Sep 17 00:00:00 2001
From: Valentin Lorentz <vlorentz@softwareheritage.org>
Date: Fri, 21 Jan 2022 14:51:18 +0100
Subject: [PATCH] Revert "Restore 'offset' and 'negative_utc' arguments and
make them optional"
This reverts commit a0f5436273ef6f5b62a388ae131ed5afa7287d00.
This means this commit removes the 'offset' and 'negative_utc'
arguments.
It also removes the 'negative_utc' attribute (which is not used
anymore), but keeps an 'offset' property, which is an alias to
'offset_minutes()'. This is only to keep this commit readable;
the next commit will remove this alias.
---
requirements.txt | 2 +-
swh/model/model.py | 192 +++++++++-------------------
swh/model/tests/test_identifiers.py | 142 +++-----------------
swh/model/tests/test_model.py | 2 +-
4 files changed, 83 insertions(+), 255 deletions(-)
diff --git a/requirements.txt b/requirements.txt
index 1e48ffba..2980e481 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,7 +1,7 @@
# Add here external Python modules dependencies, one per line. Module names
# should match https://pypi.python.org/pypi names. For the full spec or
# dependency lines, see https://pip.readthedocs.org/en/1.1/requirements.html
-attrs >= 21.1.1
+attrs != 21.1.0 # https://github.com/python-attrs/attrs/issues/804
attrs_strict >= 0.0.7
deprecated
hypothesis
diff --git a/swh/model/model.py b/swh/model/model.py
index b452fcf6..0247e03e 100644
--- a/swh/model/model.py
+++ b/swh/model/model.py
@@ -380,7 +380,7 @@ class Timestamp(BaseModel):
raise ValueError("Microseconds must be in [0, 1000000[.")
-@attr.s(frozen=True, slots=True, init=False)
+@attr.s(frozen=True, slots=True)
class TimestampWithTimezone(BaseModel):
"""Represents a TZ-aware timestamp from a VCS."""
@@ -388,9 +388,6 @@ class TimestampWithTimezone(BaseModel):
timestamp = attr.ib(type=Timestamp, validator=type_validator())
- offset = attr.ib(type=int, validator=type_validator())
- negative_utc = attr.ib(type=bool, validator=type_validator())
-
offset_bytes = attr.ib(type=bytes, validator=type_validator())
"""Raw git representation of the timezone, as an offset from UTC.
It should follow this format: ``+HHMM`` or ``-HHMM`` (including ``+0000`` and
@@ -400,123 +397,6 @@ class TimestampWithTimezone(BaseModel):
original objects, so it may differ from this format when they do.
"""
- def __init__(
- self,
- timestamp: Timestamp,
- offset: int = None,
- negative_utc: bool = None,
- offset_bytes: bytes = None,
- ):
- if offset_bytes is None:
- if offset is None:
- raise AttributeError("Neither 'offset' nor 'offset_bytes' was passed.")
- if negative_utc is None:
- raise AttributeError(
- "Neither 'negative_utc' nor 'offset_bytes' was passed."
- )
- negative = offset < 0 or negative_utc
- (hours, minutes) = divmod(abs(offset), 60)
- offset_bytes = f"{'-' if negative else '+'}{hours:02}{minutes:02}".encode()
- else:
- offset = self._parse_offset_bytes(offset_bytes)
- negative_utc = offset == 0 and offset_bytes.startswith(b"-")
-
- self.__attrs_init__( # type: ignore
- timestamp=timestamp,
- offset=offset,
- negative_utc=negative_utc,
- offset_bytes=offset_bytes,
- )
-
- @offset.validator
- def check_offset(self, attribute, value):
- """Checks the offset is a 16-bits signed integer (in theory, it
- should always be between -14 and +14 hours)."""
- if not (-(2 ** 15) <= value < 2 ** 15):
- # max 14 hours offset in theory, but you never know what
- # you'll find in the wild...
- raise ValueError("offset too large: %d minutes" % value)
-
- self._check_offsets_match()
-
- @negative_utc.validator
- def check_negative_utc(self, attribute, value):
- if self.offset and value:
- raise ValueError("negative_utc can only be True is offset=0")
-
- self._check_offsets_match()
-
- @offset_bytes.validator
- def check_offset_bytes(self, attribute, value):
- if not set(value) <= _OFFSET_CHARS:
- raise ValueError(f"invalid characters in offset_bytes: {value!r}")
-
- self._check_offsets_match()
-
- @staticmethod
- def _parse_offset_bytes(offset_bytes: bytes) -> int:
- """Parses an ``offset_bytes`` value (in Git's ``[+-]HHMM`` format),
- and returns the corresponding numeric values (in number of minutes).
-
- Tries to account for some mistakes in the format, to support incorrect
- Git implementations.
-
- >>> TimestampWithTimezone._parse_offset_bytes(b"+0000")
- 0
- >>> TimestampWithTimezone._parse_offset_bytes(b"-0000")
- 0
- >>> TimestampWithTimezone._parse_offset_bytes(b"+0200")
- 120
- >>> TimestampWithTimezone._parse_offset_bytes(b"-0200")
- -120
- >>> TimestampWithTimezone._parse_offset_bytes(b"+200")
- 120
- >>> TimestampWithTimezone._parse_offset_bytes(b"-200")
- -120
- >>> TimestampWithTimezone._parse_offset_bytes(b"+02")
- 120
- >>> TimestampWithTimezone._parse_offset_bytes(b"-02")
- -120
- >>> TimestampWithTimezone._parse_offset_bytes(b"+0010")
- 10
- >>> TimestampWithTimezone._parse_offset_bytes(b"-0010")
- -10
- >>> TimestampWithTimezone._parse_offset_bytes(b"+200000000000000000")
- 0
- >>> TimestampWithTimezone._parse_offset_bytes(b"+0160") # 60 minutes...
- 0
- """
- offset_str = offset_bytes.decode()
- assert offset_str[0] in "+-"
- sign = int(offset_str[0] + "1")
- if len(offset_str) <= 3:
- hours = int(offset_str[1:])
- minutes = 0
- else:
- hours = int(offset_str[1:-2])
- minutes = int(offset_str[-2:])
-
- offset = sign * (hours * 60 + minutes)
- if (0 <= minutes <= 59) and (-(2 ** 15) <= offset < 2 ** 15):
- return offset
- else:
- # can't parse it to a reasonable value; give up and pretend it's UTC.
- return 0
-
- def _check_offsets_match(self):
- offset = self._parse_offset_bytes(self.offset_bytes)
- if offset != self.offset:
- raise ValueError(
- f"offset_bytes ({self.offset_bytes!r}) does not match offset "
- f"{divmod(self.offset, 60)}"
- )
-
- if offset == 0 and self.negative_utc != self.offset_bytes.startswith(b"-"):
- raise ValueError(
- f"offset_bytes ({self.offset_bytes!r}) does not match negative_utc "
- f"({self.negative_utc})"
- )
-
@classmethod
def from_numeric_offset(
cls, timestamp: Timestamp, offset: int, negative_utc: bool
@@ -527,12 +407,7 @@ class TimestampWithTimezone(BaseModel):
negative = offset < 0 or negative_utc
(hours, minutes) = divmod(abs(offset), 60)
offset_bytes = f"{'-' if negative else '+'}{hours:02}{minutes:02}".encode()
- tstz = TimestampWithTimezone(
- timestamp=timestamp,
- offset_bytes=offset_bytes,
- offset=offset,
- negative_utc=negative_utc,
- )
+ tstz = TimestampWithTimezone(timestamp=timestamp, offset_bytes=offset_bytes)
assert tstz.offset == offset, (tstz.offset, offset)
return tstz
@@ -561,7 +436,7 @@ class TimestampWithTimezone(BaseModel):
timestamp = Timestamp(seconds=seconds, microseconds=microseconds)
if "offset_bytes" in time_representation:
- return TimestampWithTimezone(
+ return cls(
timestamp=timestamp,
offset_bytes=time_representation["offset_bytes"],
)
@@ -595,7 +470,7 @@ class TimestampWithTimezone(BaseModel):
# TODO: warn when using from_dict() on an int
seconds = time_representation
timestamp = Timestamp(seconds=time_representation, microseconds=0)
- return TimestampWithTimezone(timestamp=timestamp, offset_bytes=b"+0000")
+ return cls(timestamp=timestamp, offset_bytes=b"+0000")
else:
raise ValueError(
f"TimestampWithTimezone.from_dict received non-integer timestamp: "
@@ -627,9 +502,59 @@ class TimestampWithTimezone(BaseModel):
tstz = cls.from_datetime(dt)
if dt.tzname() == "-00:00":
assert tstz.offset_bytes == b"+0000"
- tstz = attr.evolve(tstz, offset_bytes=b"-0000", negative_utc=True)
+ tstz = attr.evolve(tstz, offset_bytes=b"-0000")
return tstz
+ @staticmethod
+ def _parse_offset_bytes(offset_bytes: bytes) -> int:
+ """Parses an ``offset_bytes`` value (in Git's ``[+-]HHMM`` format),
+ and returns the corresponding numeric values (in number of minutes).
+
+ Tries to account for some mistakes in the format, to support incorrect
+ Git implementations.
+
+ >>> TimestampWithTimezone._parse_offset_bytes(b"+0000")
+ 0
+ >>> TimestampWithTimezone._parse_offset_bytes(b"-0000")
+ 0
+ >>> TimestampWithTimezone._parse_offset_bytes(b"+0200")
+ 120
+ >>> TimestampWithTimezone._parse_offset_bytes(b"-0200")
+ -120
+ >>> TimestampWithTimezone._parse_offset_bytes(b"+200")
+ 120
+ >>> TimestampWithTimezone._parse_offset_bytes(b"-200")
+ -120
+ >>> TimestampWithTimezone._parse_offset_bytes(b"+02")
+ 120
+ >>> TimestampWithTimezone._parse_offset_bytes(b"-02")
+ -120
+ >>> TimestampWithTimezone._parse_offset_bytes(b"+0010")
+ 10
+ >>> TimestampWithTimezone._parse_offset_bytes(b"-0010")
+ -10
+ >>> TimestampWithTimezone._parse_offset_bytes(b"+200000000000000000")
+ 0
+ >>> TimestampWithTimezone._parse_offset_bytes(b"+0160") # 60 minutes...
+ 0
+ """
+ offset_str = offset_bytes.decode()
+ assert offset_str[0] in "+-"
+ sign = int(offset_str[0] + "1")
+ if len(offset_str) <= 3:
+ hours = int(offset_str[1:])
+ minutes = 0
+ else:
+ hours = int(offset_str[1:-2])
+ minutes = int(offset_str[-2:])
+
+ offset = sign * (hours * 60 + minutes)
+ if (0 <= minutes <= 59) and (-(2 ** 15) <= offset < 2 ** 15):
+ return offset
+ else:
+ # can't parse it to a reasonable value; give up and pretend it's UTC.
+ return 0
+
def offset_minutes(self):
"""Returns the offset, as a number of minutes since UTC.
@@ -650,7 +575,12 @@ class TimestampWithTimezone(BaseModel):
... ).offset_minutes()
330
"""
- return self.offset
+ return self._parse_offset_bytes(self.offset_bytes)
+
+ @property
+ def offset(self):
+ """Deprecated alias of :meth:`offset_minutes`."""
+ return self.offset_minutes()
@attr.s(frozen=True, slots=True)
diff --git a/swh/model/tests/test_identifiers.py b/swh/model/tests/test_identifiers.py
index 5ad08d61..6214584f 100644
--- a/swh/model/tests/test_identifiers.py
+++ b/swh/model/tests/test_identifiers.py
@@ -21,7 +21,6 @@ from swh.model.model import (
Release,
Revision,
Snapshot,
- Timestamp,
TimestampWithTimezone,
)
@@ -1036,104 +1035,51 @@ TS_DICTS = [
# with current input dict format (offset_bytes)
(
{"timestamp": 12345, "offset_bytes": b"+0000"},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"+0000",
- "offset": 0,
- "negative_utc": False,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"+0000",},
),
(
{"timestamp": 12345, "offset_bytes": b"-0000"},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"-0000",
- "offset": 0,
- "negative_utc": True,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"-0000",},
),
(
{"timestamp": 12345, "offset_bytes": b"+0200"},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"+0200",
- "offset": 120,
- "negative_utc": False,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"+0200",},
),
(
{"timestamp": 12345, "offset_bytes": b"-0200"},
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"-0200",},
+ ),
+ (
+ {"timestamp": 12345, "offset_bytes": b"--700"},
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"--700",},
+ ),
+ (
+ {"timestamp": 12345, "offset_bytes": b"1234567"},
{
"timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"-0200",
- "offset": -120,
- "negative_utc": False,
+ "offset_bytes": b"1234567",
},
),
- # not working yet:
- # (
- # {"timestamp": 12345, "offset_bytes": b"--700"},
- # {
- # "timestamp": {"seconds": 12345, "microseconds": 0},
- # "offset_bytes": b"--700",
- # "offset": 0,
- # "negative_utc": False,
- # },
- # ),
- # (
- # {"timestamp": 12345, "offset_bytes": b"1234567"},
- # {
- # "timestamp": {"seconds": 12345, "microseconds": 0},
- # "offset_bytes": b"1234567",
- # "offset": 0,
- # "negative_utc": False,
- # },
- # ),
# with old-style input dicts (numeric offset + optional negative_utc):
(
{"timestamp": 12345, "offset": 0},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"+0000",
- "offset": 0,
- "negative_utc": False,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"+0000",},
),
(
{"timestamp": 12345, "offset": 0, "negative_utc": False},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"+0000",
- "offset": 0,
- "negative_utc": False,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"+0000",},
),
(
{"timestamp": 12345, "offset": 0, "negative_utc": False},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"+0000",
- "offset": 0,
- "negative_utc": False,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"+0000",},
),
(
{"timestamp": 12345, "offset": 0, "negative_utc": None},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"+0000",
- "offset": 0,
- "negative_utc": False,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"+0000",},
),
(
{"timestamp": {"seconds": 12345}, "offset": 0, "negative_utc": None},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"+0000",
- "offset": 0,
- "negative_utc": False,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"+0000",},
),
(
{
@@ -1141,12 +1087,7 @@ TS_DICTS = [
"offset": 0,
"negative_utc": None,
},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"+0000",
- "offset": 0,
- "negative_utc": False,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"+0000",},
),
(
{
@@ -1157,27 +1098,15 @@ TS_DICTS = [
{
"timestamp": {"seconds": 12345, "microseconds": 100},
"offset_bytes": b"+0000",
- "offset": 0,
- "negative_utc": False,
},
),
(
{"timestamp": 12345, "offset": 0, "negative_utc": True},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"-0000",
- "offset": 0,
- "negative_utc": True,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"-0000",},
),
(
{"timestamp": 12345, "offset": 0, "negative_utc": None},
- {
- "timestamp": {"seconds": 12345, "microseconds": 0},
- "offset_bytes": b"+0000",
- "offset": 0,
- "negative_utc": False,
- },
+ {"timestamp": {"seconds": 12345, "microseconds": 0}, "offset_bytes": b"+0000",},
),
]
@@ -1187,35 +1116,6 @@ def test_normalize_timestamp_dict(dict_input, expected):
assert TimestampWithTimezone.from_dict(dict_input).to_dict() == expected
-def test_timestampwithtimezone_init():
- ts = Timestamp(seconds=1234567, microseconds=0)
- tstz = TimestampWithTimezone(
- timestamp=ts, offset=120, negative_utc=False, offset_bytes=b"+0200"
- )
- assert tstz.timestamp == ts
- assert tstz.offset == 120
- assert tstz.negative_utc is False
- assert tstz.offset_bytes == b"+0200"
-
- assert tstz == TimestampWithTimezone(timestamp=ts, offset=120, negative_utc=False)
- assert tstz == TimestampWithTimezone(timestamp=ts, offset_bytes=b"+0200")
-
- assert tstz != TimestampWithTimezone(timestamp=ts, offset_bytes=b"+0100")
-
- tstz = TimestampWithTimezone(
- timestamp=ts, offset=0, negative_utc=True, offset_bytes=b"-0000"
- )
- assert tstz.timestamp == ts
- assert tstz.offset == 0
- assert tstz.negative_utc is True
- assert tstz.offset_bytes == b"-0000"
-
- assert tstz == TimestampWithTimezone(timestamp=ts, offset=0, negative_utc=True)
- assert tstz == TimestampWithTimezone(timestamp=ts, offset_bytes=b"-0000")
-
- assert tstz != TimestampWithTimezone(timestamp=ts, offset_bytes=b"+0000")
-
-
TS_DICTS_INVALID_TIMESTAMP = [
{"timestamp": 1.2, "offset": 0},
{"timestamp": "1", "offset": 0},
@@ -1262,8 +1162,6 @@ def test_normalize_timestamp_datetime(
assert TimestampWithTimezone.from_dict(date).to_dict() == {
"timestamp": {"seconds": seconds, "microseconds": microsecond},
"offset_bytes": offset_bytes,
- "offset": offset,
- "negative_utc": False,
}
diff --git a/swh/model/tests/test_model.py b/swh/model/tests/test_model.py
index 8e191241..9c1541e3 100644
--- a/swh/model/tests/test_model.py
+++ b/swh/model/tests/test_model.py
@@ -498,7 +498,7 @@ def test_timestampwithtimezone():
with pytest.raises(AttributeTypeError):
TimestampWithTimezone(timestamp=datetime.datetime.now(), offset_bytes=b"+0000")
- with pytest.raises((AttributeTypeError, AttributeError, TypeError)):
+ with pytest.raises((AttributeTypeError, TypeError)):
TimestampWithTimezone(timestamp=ts, offset_bytes=0)
--
GitLab