From 40149901b2cd04094a06d5692c4821ff034e5aaf Mon Sep 17 00:00:00 2001 From: Jon Cluce Date: Thu, 9 Oct 2025 15:18:25 -0400 Subject: [PATCH 1/2] :bug: Fix logging copy datetime bug --- CPAC/utils/monitoring/monitoring.py | 77 +++++++++++++++++++++++++---- CPAC/utils/tests/test_utils.py | 41 ++++++++++++++- 2 files changed, 107 insertions(+), 11 deletions(-) diff --git a/CPAC/utils/monitoring/monitoring.py b/CPAC/utils/monitoring/monitoring.py index 8d715b82b..66e231eec 100644 --- a/CPAC/utils/monitoring/monitoring.py +++ b/CPAC/utils/monitoring/monitoring.py @@ -84,7 +84,7 @@ def isoformat(self) -> str: class DatetimeWithSafeNone(datetime, _NoTime): - """Time class that can be None or a time value. + r"""Time class that can be None or a time value. Examples -------- @@ -148,6 +148,7 @@ def __new__( fold: Optional[int] = 0, ) -> "DatetimeWithSafeNone | _NoTime": """Create a new instance of the class.""" + # First check if all arguments are provided as integers if ( isinstance(year, int) and isinstance(month, int) @@ -170,10 +171,13 @@ def __new__( tzinfo, fold=fold, ) - else: - dt = year + + # Otherwise, year contains the datetime-like object + dt = year + if dt is None: return NoTime + if isinstance(dt, datetime): return datetime.__new__( cls, @@ -186,6 +190,7 @@ def __new__( dt.microsecond, dt.tzinfo, ) + if isinstance(dt, bytes): try: tzflag: Optional[int] @@ -219,25 +224,77 @@ def __new__( return datetime.__new__( cls, year, month, day, hour, minute, second, microsecond, tzinfo ) - else: - msg = f"Unexpected type: {[type(part) for part in [year, month, day, hour, minute, second, microsecond]]}" - raise TypeError(msg) - except UnicodeDecodeError: - error = f"Cannot decode bytes to string: {dt!r}" + msg = f"Unexpected type: {[type(part) for part in [year, month, day, hour, minute, second, microsecond]]}" + raise TypeError(msg) + except (struct.error, IndexError) as e: + error = f"Cannot unpack bytes to datetime: {dt!r} - {e}" raise TypeError(error) + if isinstance(dt, str): try: return DatetimeWithSafeNone(datetime.fromisoformat(dt)) except (ValueError, TypeError): error = f"Invalid ISO-format datetime string: {dt}" - else: - error = f"Cannot convert {type(dt)} to datetime" + raise TypeError(error) + + error = f"Cannot convert {type(dt)} to datetime" raise TypeError(error) def __bool__(self) -> bool: """Return True if not NoTime.""" return self is not NoTime + def __eq__(self, other: object) -> bool: + """Compare DatetimeWithSafeNone instances with tzinfo-aware logic. + + If only one side has tzinfo, consider them equal if all other components match. + """ + if self is NoTime and other is NoTime: + return True + if self is NoTime or other is NoTime: + return False + if not isinstance(other, (datetime, DatetimeWithSafeNone)): + return False + + # Compare all datetime components except tzinfo + components_match = ( + self.year == other.year + and self.month == other.month + and self.day == other.day + and self.hour == other.hour + and self.minute == other.minute + and self.second == other.second + and self.microsecond == other.microsecond + ) + + if not components_match: + return False + + # If components match, check tzinfo: + # - If either has None tzinfo, consider them equal + # - If both have tzinfo, they must match + if self.tzinfo is None or other.tzinfo is None: + return True + + return self.tzinfo == other.tzinfo + + def __hash__(self) -> int: + """Return hash based on datetime components, ignoring tzinfo.""" + if self is NoTime: + return hash(NoTime) + # Hash based on datetime components only, not tzinfo + return hash( + ( + self.year, + self.month, + self.day, + self.hour, + self.minute, + self.second, + self.microsecond, + ) + ) + def __sub__(self, other: "DatetimeWithSafeNone | _NoTime") -> datetime | timedelta: # type: ignore[reportIncompatibleMethodOverride] """Subtract between a datetime or timedelta or None.""" return _safe_none_diff(self, other) diff --git a/CPAC/utils/tests/test_utils.py b/CPAC/utils/tests/test_utils.py index 6c9d11104..6014103fb 100644 --- a/CPAC/utils/tests/test_utils.py +++ b/CPAC/utils/tests/test_utils.py @@ -16,7 +16,8 @@ # License along with C-PAC. If not, see . """Tests of CPAC utility functions.""" -from datetime import datetime, timedelta +from copy import deepcopy +from datetime import datetime, timedelta, timezone import multiprocessing from unittest import mock @@ -240,3 +241,41 @@ def test_datetime_with_safe_none(t1: OptionalDatetime, t2: OptionalDatetime): assert isinstance(t2 - t1, timedelta) else: assert t2 - t1 == timedelta(0) + + +def test_deepcopy_datetimewithsafenone_raises_error() -> None: + """Test bytestring TypeError during deepcopy operation.""" + # Create a node dictionary similar to what's used in the Gantt chart generation + node = { + "id": "test_node", + "hash": "abc123", + "start": DatetimeWithSafeNone( + datetime(2024, 1, 1, 10, 0, 0, tzinfo=timezone.utc) + ), + "finish": DatetimeWithSafeNone( + datetime(2024, 1, 1, 11, 30, 0, tzinfo=timezone.utc) + ), + "runtime_threads": 4, + "runtime_memory_gb": 2.5, + "estimated_memory_gb": 3.0, + "num_threads": 4, + } + + # This should raise: TypeError: Cannot convert to datetime + # with the original code because deepcopy pickles DatetimeWithSafeNone objects + # as bytes, and the __new__ method doesn't properly handle the pickle protocol + finish_node = deepcopy(node) + + assert finish_node["start"] == node["start"] + assert finish_node["finish"] == node["finish"] + + +def test_deepcopy_datetimewithsafenone_direct(): + """Test deepcopy directly on DatetimeWithSafeNone instance.""" + dt = DatetimeWithSafeNone(datetime(2024, 1, 1, 10, 0, 0, tzinfo=timezone.utc)) + + # This triggers the pickle/unpickle cycle which passes bytes to __new__ + dt_copy = deepcopy(dt) + + assert dt_copy == dt + assert isinstance(dt_copy, DatetimeWithSafeNone) From 2ab43b12fefda59ac1d4021badd729a5a88d3a60 Mon Sep 17 00:00:00 2001 From: Jon Cluce Date: Fri, 10 Oct 2025 13:13:40 -0400 Subject: [PATCH 2/2] :pencil2: Fix escapes in rawstring --- CPAC/utils/monitoring/monitoring.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CPAC/utils/monitoring/monitoring.py b/CPAC/utils/monitoring/monitoring.py index 66e231eec..632ecc4a6 100644 --- a/CPAC/utils/monitoring/monitoring.py +++ b/CPAC/utils/monitoring/monitoring.py @@ -93,9 +93,9 @@ class DatetimeWithSafeNone(datetime, _NoTime): '2025-06-18T21:06:43.730004' >>> DatetimeWithSafeNone("2025-06-18T21:06:43.730004").isoformat() '2025-06-18T21:06:43.730004' - >>> DatetimeWithSafeNone(b"\\x07\\xe9\\x06\\x12\\x10\\x18\\x1c\\x88\\x6d\\x01").isoformat() + >>> DatetimeWithSafeNone(b"\x07\xe9\x06\x12\x10\x18\x1c\x88\x6d\x01").isoformat() '2025-06-18T16:24:28.028040+00:00' - >>> DatetimeWithSafeNone(b'\\x07\\xe9\\x06\\x12\\x10\\x18\\x1c\\x88m\\x00').isoformat() + >>> DatetimeWithSafeNone(b'\x07\xe9\x06\x12\x10\x18\x1c\x88m\x00').isoformat() '2025-06-18T16:24:28.028040' >>> DatetimeWithSafeNone(DatetimeWithSafeNone("2025-06-18")).isoformat() '2025-06-18T00:00:00'