Raise an exception when comparing instance with differing framerates

Use NotImplemented for undefined comparisons to unknown types
to allow other classes the chance to provide an implementation
Closes  #62

Author: nickswalker

tests/test_timecode.py

@@ -407,21 +407,31 @@ def test_add_with_two_different_frame_rates():
     "args,kwargs,func,tc2", [
         [["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_add_with_non_suitable_class_instance(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."""
@@ -973,6 +983,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

timecode/__init__.py

@@ -588,14 +588,16 @@ def __eq__(self, other: Union[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
         elif isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.__eq__(new_tc)
         elif isinstance(other, int):
             return self.frames == other
-        else: 
-            return False
+        else:
+            return NotImplemented
 
     def __ge__(self, other: Union[int, str, "Timecode", object]) -> bool:
         """Override greater than or equal to operator.
@@ -610,16 +612,16 @@ def __ge__(self, other: Union[int, str, "Timecode", object]) -> bool:
             bool: True if the other is greater 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
         elif isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.frames >= new_tc.frames
         elif isinstance(other, int):
             return self.frames >= other
         else:
-            raise TypeError(
-                "'>=' not supported between instances of 'Timecode' and '{}'".format(other.__class__.__name__)
-            )
+            return NotImplemented
 
     def __gt__(self, other: Union[int, str, "Timecode"]) -> bool:
         """Override greater than operator.
@@ -634,16 +636,16 @@ def __gt__(self, other: Union[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
         elif isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.frames > new_tc.frames
         elif isinstance(other, int):
             return self.frames > other
         else:
-            raise TypeError(
-                "'>' not supported between instances of 'Timecode' and '{}'".format(other.__class__.__name__)
-            )
+            return NotImplemented
 
     def __le__(self, other: Union[int, str, "Timecode", object]) -> bool:
         """Override less or equal to operator.
@@ -658,16 +660,16 @@ def __le__(self, other: Union[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
         elif isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.frames <= new_tc.frames
         elif isinstance(other, int):
             return self.frames <= other
         else:
-            raise TypeError(
-                "'<' not supported between instances of 'Timecode' and '{}'".format(other.__class__.__name__)
-            )
+            return NotImplemented
 
     def __lt__(self, other: Union[int, str, "Timecode"]) -> bool:
         """Override less than operator.
@@ -682,16 +684,16 @@ def __lt__(self, other: Union[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".format())
             return self.framerate == other.framerate and self.frames < other.frames
         elif isinstance(other, str):
             new_tc = Timecode(self.framerate, other)
             return self.frames < new_tc.frames
         elif isinstance(other, int):
             return self.frames < other
         else:
-            raise TypeError(
-                "'<=' not supported between instances of 'Timecode' and '{}'".format(other.__class__.__name__)
-            )
+            return NotImplemented
 
     def __add__(self, other: Union["Timecode", int]) -> "Timecode":
         """Return a new Timecode with the given timecode or frames added to this one.
@@ -715,9 +717,7 @@ def __add__(self, other: Union["Timecode", int]) -> "Timecode":
         elif isinstance(other, int):
             tc.add_frames(other)
         else:
-            raise TimecodeError(
-                "Type {} not supported for arithmetic.".format(other.__class__.__name__)
-            )
+            return NotImplemented
 
         return tc
 
@@ -739,9 +739,7 @@ def __sub__(self, other: Union["Timecode", int]) -> "Timecode":
         elif isinstance(other, int):
             subtracted_frames = self.frames - other
         else:
-            raise TimecodeError(
-                "Type {} not supported for arithmetic.".format(other.__class__.__name__)
-            )
+            return NotImplemented
         tc = Timecode(self.framerate, frames=abs(subtracted_frames))
         tc.drop_frame = self.drop_frame
         return tc
@@ -764,9 +762,7 @@ def __mul__(self, other: Union["Timecode", int]) -> "Timecode":
         elif isinstance(other, int):
             multiplied_frames = self.frames * other
         else:
-            raise TimecodeError(
-                "Type {} not supported for arithmetic.".format(other.__class__.__name__)
-            )
+            return NotImplemented
         tc = Timecode(self.framerate, frames=multiplied_frames)
         tc.drop_frame = self.drop_frame
         return tc
@@ -789,9 +785,7 @@ def __div__(self, other: Union["Timecode", int]) -> "Timecode":
         elif isinstance(other, int):
             div_frames = int(float(self.frames) / float(other))
         else:
-            raise TimecodeError(
-                "Type {} not supported for arithmetic.".format(other.__class__.__name__)
-            )
+            return NotImplemented
 
         return Timecode(self.framerate, frames=div_frames)