From 0a516cfe3b1e50cae477e925f2cd4b749f320e9a Mon Sep 17 00:00:00 2001 From: Nick Walker Date: Wed, 9 Jul 2025 00:43:23 -0700 Subject: [PATCH 1/3] 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 eoyilmaz/timecode#62 # Conflicts: # src/timecode/timecode.py # tests/test_timecode.py --- src/timecode/timecode.py | 58 +++++++++++++++++----------------------- tests/test_timecode.py | 32 +++++++++++++++++----- 2 files changed, 49 insertions(+), 41 deletions(-) diff --git a/src/timecode/timecode.py b/src/timecode/timecode.py index 5b5cd04..12dc21c 100644 --- a/src/timecode/timecode.py +++ b/src/timecode/timecode.py @@ -609,13 +609,15 @@ 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. @@ -631,16 +633,15 @@ 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. @@ -655,16 +656,15 @@ 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. @@ -679,16 +679,15 @@ 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 isinstance(other, str): + 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 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. @@ -703,16 +702,15 @@ 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".format()) 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. @@ -736,9 +734,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 @@ -760,9 +756,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 @@ -785,9 +779,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 @@ -810,9 +802,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) diff --git a/tests/test_timecode.py b/tests/test_timecode.py index f6ccbfe..b9ffe0a 100644 --- a/tests/test_timecode.py +++ b/tests/test_timecode.py @@ -471,21 +471,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_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.""" @@ -1053,6 +1063,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 From bb145676d7f30a976efe52fe26d33953e1caa5d5 Mon Sep 17 00:00:00 2001 From: Nick Walker Date: Tue, 30 Dec 2025 19:18:27 -0500 Subject: [PATCH 2/3] Fix lint issues --- src/timecode/timecode.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/timecode/timecode.py b/src/timecode/timecode.py index 12dc21c..ff58f12 100644 --- a/src/timecode/timecode.py +++ b/src/timecode/timecode.py @@ -610,7 +610,8 @@ def __eq__(self, other: int | str | Timecode | object) -> bool: """ if isinstance(other, Timecode): if self.framerate != other.framerate: - raise ValueError("'==' not supported between instances of 'Timecode' with different framerates") + 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) @@ -634,7 +635,8 @@ def __ge__(self, other: int | str | Timecode | object) -> bool: """ if isinstance(other, Timecode): if self.framerate != other.framerate: - raise ValueError("'>=' not supported between instances of 'Timecode' with different framerates") + 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) @@ -657,7 +659,8 @@ def __gt__(self, other: int | str | Timecode) -> bool: """ if isinstance(other, Timecode): if self.framerate != other.framerate: - raise ValueError("'>' not supported between instances of 'Timecode' with different framerates") + 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) @@ -680,9 +683,10 @@ def __le__(self, other: int | str | Timecode | object) -> bool: """ if isinstance(other, Timecode): if self.framerate != other.framerate: - raise ValueError("'<=' not supported between instances of 'Timecode' with different framerates") + raise ValueError("'<=' not supported between instances of " + "'Timecode' with different framerates") return self.frames <= other.frames - elif isinstance(other, str): + if isinstance(other, str): new_tc = Timecode(self.framerate, other) return self.frames <= new_tc.frames if isinstance(other, int): @@ -703,7 +707,8 @@ def __lt__(self, other: int | str | Timecode) -> bool: """ if isinstance(other, Timecode): if self.framerate != other.framerate: - raise ValueError("'<' not supported between instances of 'Timecode' with different framerates".format()) + 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) From 9515f9e358cacd91c7886e7bc6412a934fb16fa2 Mon Sep 17 00:00:00 2001 From: Nick Walker Date: Sun, 28 Dec 2025 17:37:38 -0700 Subject: [PATCH 3/3] Remove test that assumed equality over different frame bases --- tests/test_timecode.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/test_timecode.py b/tests/test_timecode.py index b9ffe0a..909083e 100644 --- a/tests/test_timecode.py +++ b/tests/test_timecode.py @@ -707,12 +707,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