Merge branch 'develop' into fix/bytes-and-strings

Author: shnizzedy

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
     --------
@@ -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'
@@ -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)

CPAC/utils/tests/test_utils.py

@@ -16,7 +16,8 @@
 # License along with C-PAC. If not, see <https://www.gnu.org/licenses/>.
 """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 <class 'bytes'> 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)