Merge pull request #64 from nickswalker/deimplement-comparison

Raise an exception when comparing instance with differing framerates

Author: eoyilmaz

src/timecode/timecode.py

@@ -610,13 +610,16 @@ def __eq__(self, other: int | str | Timecode | object) -> bool:
             bool: True if the other is equal to this Timecode instance.
         """
         if isinstance(other, Timecode):
-            return self.framerate == other.framerate and self.frames == other.frames
+            if self.framerate != other.framerate:
+                raise ValueError("'==' not supported between instances of "
+                                 "'Timecode' with different framerates")
+            return self.frames == other.frames
         if isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.__eq__(new_tc)
         if isinstance(other, int):
             return self.frames == other
-        return False
+        return NotImplemented
 
     def __ge__(self, other: int | str | Timecode | object) -> bool:
         """Override greater than or equal to operator.
@@ -632,16 +635,16 @@ def __ge__(self, other: int | str | Timecode | object) -> bool:
                 instance.
         """
         if isinstance(other, Timecode):
-            return self.framerate == other.framerate and self.frames >= other.frames
+            if self.framerate != other.framerate:
+                raise ValueError("'>=' not supported between instances of "
+                                 "'Timecode' with different framerates")
+            return self.frames >= other.frames
         if isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.frames >= new_tc.frames
         if isinstance(other, int):
             return self.frames >= other
-        raise TypeError(
-            "'>=' not supported between instances of 'Timecode' and "
-            f"'{other.__class__.__name__}'"
-        )
+        return NotImplemented
 
     def __gt__(self, other: int | str | Timecode) -> bool:
         """Override greater than operator.
@@ -656,16 +659,16 @@ def __gt__(self, other: int | str | Timecode) -> bool:
             bool: True if the other is greater than this Timecode instance.
         """
         if isinstance(other, Timecode):
-            return self.framerate == other.framerate and self.frames > other.frames
+            if self.framerate != other.framerate:
+                raise ValueError("'>' not supported between instances of "
+                                 "'Timecode' with different framerates")
+            return self.frames > other.frames
         if isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.frames > new_tc.frames
         if isinstance(other, int):
             return self.frames > other
-        raise TypeError(
-            "'>' not supported between instances of 'Timecode' and "
-            f"'{other.__class__.__name__}'"
-        )
+        return NotImplemented
 
     def __le__(self, other: int | str | Timecode | object) -> bool:
         """Override less or equal to operator.
@@ -680,16 +683,16 @@ def __le__(self, other: int | str | Timecode | object) -> bool:
             bool: True if the other is less than or equal to this Timecode instance.
         """
         if isinstance(other, Timecode):
-            return self.framerate == other.framerate and self.frames <= other.frames
+            if self.framerate != other.framerate:
+                raise ValueError("'<=' not supported between instances of "
+                                 "'Timecode' with different framerates")
+            return self.frames <= other.frames
         if isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.frames <= new_tc.frames
         if isinstance(other, int):
             return self.frames <= other
-        raise TypeError(
-            "'<' not supported between instances of 'Timecode' and "
-            f"'{other.__class__.__name__}'"
-        )
+        return NotImplemented
 
     def __lt__(self, other: int | str | Timecode) -> bool:
         """Override less than operator.
@@ -704,16 +707,16 @@ def __lt__(self, other: int | str | Timecode) -> bool:
             bool: True if the other is less than this Timecode instance.
         """
         if isinstance(other, Timecode):
+            if self.framerate != other.framerate:
+                raise ValueError("'<' not supported between instances of "
+                                 "'Timecode' with different framerates")
             return self.framerate == other.framerate and self.frames < other.frames
         if isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.frames < new_tc.frames
         if isinstance(other, int):
             return self.frames < other
-        raise TypeError(
-            "'<=' not supported between instances of 'Timecode' and "
-            f"'{other.__class__.__name__}'"
-        )
+        return NotImplemented
 
     def __add__(self, other: int | Timecode) -> Timecode:
         """Return a new Timecode with the given timecode or frames added to this one.
@@ -737,9 +740,7 @@ def __add__(self, other: int | Timecode) -> Timecode:
         elif isinstance(other, int):
             tc.add_frames(other)
         else:
-            raise TimecodeError(
-                f"Type {other.__class__.__name__} not supported for arithmetic."
-            )
+            return NotImplemented
 
         return tc
 
@@ -761,9 +762,7 @@ def __sub__(self, other: int | Timecode) -> Timecode:
         elif isinstance(other, int):
             subtracted_frames = self.frames - other
         else:
-            raise TimecodeError(
-                f"Type {other.__class__.__name__} not supported for arithmetic."
-            )
+            return NotImplemented
         tc = Timecode(self.framerate, frames=abs(subtracted_frames))
         tc.drop_frame = self.drop_frame
         return tc
@@ -786,9 +785,7 @@ def __mul__(self, other: int | Timecode) -> Timecode:
         elif isinstance(other, int):
             multiplied_frames = self.frames * other
         else:
-            raise TimecodeError(
-                f"Type {other.__class__.__name__} not supported for arithmetic."
-            )
+            return NotImplemented
         tc = Timecode(self.framerate, frames=multiplied_frames)
         tc.drop_frame = self.drop_frame
         return tc
@@ -811,9 +808,7 @@ def __div__(self, other: int | Timecode) -> Timecode:
         elif isinstance(other, int):
             div_frames = int(float(self.frames) / float(other))
         else:
-            raise TimecodeError(
-                f"Type {other.__class__.__name__} not supported for arithmetic."
-            )
+            return NotImplemented
 
         return Timecode(self.framerate, frames=div_frames)
 

tests/test_timecode.py

@@ -1018,21 +1018,31 @@ def test_add_with_two_different_frame_rates():
     [
         [["24", "00:00:01:00"], {}, lambda x, y: x + y, "not suitable"],
         [["24", "00:00:01:00"], {}, lambda x, y: x - y, "not suitable"],
-        [["24", "00:00:01:00"], {}, lambda x, y: x * y, "not suitable"],
         [["24", "00:00:01:00"], {}, lambda x, y: x / y, "not suitable"],
         [["24", "00:00:01:00"], {}, lambda x, y: x / y, 32.4],
-    ],
+        [["24", "00:00:01:00"], {}, lambda x, y: x * y, 32.4],
+    ]
 )
-def test_arithmetic_with_unsupported_type_raises_error(args, kwargs, func, tc2):
-    """TimecodeError is raised if the other class is not suitable for the operation."""
+def test_arithmetic_with_non_suitable_class_instance(args, kwargs, func, tc2):
+    """TypeError is raised if the other class is not suitable for the operation."""
     tc1 = Timecode(*args, **kwargs)
-    with pytest.raises(TimecodeError) as cm:
+    with pytest.raises(TypeError) as cm:
         _ = func(tc1, tc2)
 
-    assert str(cm.value) == "Type {} not supported for arithmetic.".format(
-        tc2.__class__.__name__
-    )
+    assert str(cm.value).startswith(f"unsupported operand type(s) for")
+    assert str(cm.value).endswith(f"'Timecode' and '{tc2.__class__.__name__}'")
 
+@pytest.mark.parametrize(
+    "args,kwargs,func,tc2", [
+        [["24", "00:00:01:00"], {}, lambda x, y: x * y, "not suitable"],
+    ]
+)
+def test_multiply_with_sequence_type(args, kwargs, func, tc2):
+    """TypeError is raised if the other class is not suitable for the operation."""
+    tc1 = Timecode(*args, **kwargs)
+    with pytest.raises(TypeError) as cm:
+        _ = func(tc1, tc2)
+    assert str(cm.value) == (f"can't multiply sequence by non-int of type 'Timecode'")
 
 def test_div_method_working_properly_1():
     """__div__ method is working properly."""
@@ -1248,12 +1258,6 @@ def test_ms_vs_fraction_frames_2():
 
 
 def test_ms_vs_fraction_frames_3():
-    tc1 = Timecode("ms", "00:00:00.040")
-    tc2 = Timecode(24, "00:00:00.042")
-    assert tc1 != tc2
-
-
-def test_ms_vs_fraction_frames_4():
     tc1 = Timecode("ms", "00:00:00.040")
     tc2 = Timecode(24, "00:00:00.042")
     assert tc1.frame_number == 40
@@ -1611,6 +1615,14 @@ def test_lt_method_with_integers():
     tc = Timecode("24", "00:00:10:00")
     assert tc < 250
 
+def test_comparison_with_different_framerates_raises():
+    """Comparing Timecodes with different framerates."""
+    tc1 = Timecode("24", "00:00:00:00")
+    tc2 = Timecode("30", "00:00:00:00")
+    with pytest.raises(ValueError) as cm:
+        _ = tc1 < tc2
+        _ = tc1 > tc2
+
 
 def test_fraction_lib_from_python3_raises_import_error_for_python2():
     """ImportError is raised and the error is handled gracefully under Python 2 if