Merge pull request #30 from bbc/jamesba-unionwithnothing

Fixed bug when taking union with an empty range

Author: jamesba

CHANGELOG.md

@@ -1,5 +1,8 @@
 # mediatimestamp Changelog
 
+## 1.5.2
+- Fixed bug in taking unions with empty ranges
+
 ## 1.5.1
 - Fixed bug in `TimeOffset.to_count` (both versions) that caused
   incorrect results when rounding down when the denominator of the

mediatimestamp/immutable.py

@@ -1026,39 +1026,49 @@ def intersect_with(self, tr):
 
     def starts_inside_timerange(self, other):
         """Returns true if the start of this timerange is located inside the other."""
-        return ((self.bounded_before() and self.start in other and
-                 (not (other.bounded_after() and self.start == other.end and not self.includes_start()))) or
-                (self.bounded_before() and other.bounded_before() and self.start == other.start and
-                 (not (self.includes_start() and not other.includes_start()))) or
-                (not self.bounded_before() and not other.bounded_before()))
+        return (not self.is_empty() and
+                not other.is_empty() and
+                ((self.bounded_before() and self.start in other and
+                  (not (other.bounded_after() and self.start == other.end and not self.includes_start()))) or
+                 (self.bounded_before() and other.bounded_before() and self.start == other.start and
+                  (not (self.includes_start() and not other.includes_start()))) or
+                 (not self.bounded_before() and not other.bounded_before())))
 
     def ends_inside_timerange(self, other):
         """Returns true if the end of this timerange is located inside the other."""
-        return ((self.bounded_after() and self.end in other and
-                 (not (other.bounded_before() and self.end == other.start and not self.includes_end()))) or
-                (self.bounded_after() and other.bounded_after() and self.end == other.end and
-                 (not (self.includes_end() and not other.includes_end()))) or
-                (not self.bounded_after() and not other.bounded_after()))
+        return (not self.is_empty() and
+                not other.is_empty() and
+                ((self.bounded_after() and self.end in other and
+                  (not (other.bounded_before() and self.end == other.start and not self.includes_end()))) or
+                 (self.bounded_after() and other.bounded_after() and self.end == other.end and
+                  (not (self.includes_end() and not other.includes_end()))) or
+                 (not self.bounded_after() and not other.bounded_after())))
 
     def is_earlier_than_timerange(self, other):
         """Returns true if this timerange ends earlier than the start of the other."""
-        return (other.bounded_before() and
+        return (not self.is_empty() and
+                not other.is_empty() and
+                other.bounded_before() and
                 self.bounded_after() and
                 (self.end < other.start or
                  (self.end == other.start and
                   not (self.includes_end() and other.includes_start()))))
 
     def is_later_than_timerange(self, other):
         """Returns true if this timerange starts later than the end of the other."""
-        return (other.bounded_after() and
+        return (not self.is_empty() and
+                not other.is_empty() and
+                other.bounded_after() and
                 self.bounded_before() and
                 (self.start > other.end or
                  (self.start == other.end and
                   not (self.includes_start() and other.includes_end()))))
 
     def starts_earlier_than_timerange(self, other):
         """Returns true if this timerange starts earlier than the start of the other."""
-        return (other.bounded_before() and
+        return (not self.is_empty() and
+                not other.is_empty() and
+                other.bounded_before() and
                 (not self.bounded_before() or
                  (self.start < other.start or
                   (self.start == other.start and
@@ -1067,23 +1077,29 @@ def starts_earlier_than_timerange(self, other):
 
     def starts_later_than_timerange(self, other):
         """Returns true if this timerange starts later than the start of the other."""
-        return (self.bounded_before() and
+        return (not self.is_empty() and
+                not other.is_empty() and
+                self.bounded_before() and
                 (not other.bounded_before() or
                  (self.start > other.start or
                   (self.start == other.start and
                    (not self.includes_start() and other.includes_start())))))
 
     def ends_earlier_than_timerange(self, other):
         """Returns true if this timerange ends earlier than the end of the other."""
-        return (self.bounded_after() and
+        return (not self.is_empty() and
+                not other.is_empty() and
+                self.bounded_after() and
                 (not other.bounded_after() or
                  (self.end < other.end or
                   (self.end == other.end and
                    (not self.includes_end() and other.includes_end())))))
 
     def ends_later_than_timerange(self, other):
         """Returns true if this timerange ends later than the end of the other."""
-        return (other.bounded_after() and
+        return (not self.is_empty() and
+                not other.is_empty() and
+                other.bounded_after() and
                 (not self.bounded_after() or
                  (self.end > other.end or
                   (self.end == other.end and
@@ -1110,6 +1126,12 @@ def union_with_timerange(self, other):
         if not self.is_contiguous_with_timerange(other):
             raise ValueError("Timeranges {} and {} are not contiguous, so cannot take the union.".format(self, other))
 
+        if self.is_empty():
+            return other
+
+        if other.is_empty():
+            return self
+
         inclusivity = TimeRange.EXCLUSIVE
         if self.start == other.start:
             start = self.start

setup.py

@@ -18,7 +18,7 @@
 
 # Basic metadata
 name = 'mediatimestamp'
-version = '1.5.1'
+version = '1.5.2'
 description = 'A timestamp library for high precision nanosecond timestamps'
 url = 'https://github.com/bbc/rd-apmm-python-lib-mediatimestamp'
 author = 'James P. Weaver'

tests/test_immutable.py

@@ -1378,6 +1378,33 @@ def test_comparisons(self):
              (False, False, True, False, True, False, True, False, False, True)),
             (TimeRange.from_str("[0:0_5:0)"), TimeRange.from_str("(5:0_10:0)"),
              (False, False, True, False, True, False, True, False, False, False)),
+
+            (TimeRange.never(), TimeRange.from_str("_"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("[0:0_"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("(0:0_"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("[10:0_"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("(10:0_"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("_0:0]"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("_0:0)"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("_10:0]"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("_10:0)"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("[0:0_10:0)"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("(0:0_10:0)"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("[5:0_10:0)"),
+             (False, False, False, False, False, False, False, False, True, True)),
+            (TimeRange.never(), TimeRange.from_str("(5:0_10:0)"),
+             (False, False, False, False, False, False, False, False, True, True)),
         ]
         functions = ("starts_inside_timerange",
                      "ends_inside_timerange",
@@ -1503,3 +1530,45 @@ def test_normalise(self):
                 self.assertEqual(result, expected,
                                  msg=("{!r}.normalise({}, {}, rounding={}) == {!r}, expected {!r}"
                                       .format(tr, rate.numerator, rate.denominator, rounding, result, expected)))
+
+    def test_union(self):
+        test_data = [
+            (TimeRange.from_str("()"), TimeRange.from_str("()"),
+             TimeRange.from_str("()")),
+            (TimeRange.from_str("[0:0_10:0)"), TimeRange.from_str("[10:0]"),
+             TimeRange.from_str("[0:0_10:0]")),
+            (TimeRange.from_str("_"), TimeRange.from_str("[0:0]"),
+             TimeRange.from_str("_")),
+            (TimeRange.from_str("_"), TimeRange.from_str("()"),
+             TimeRange.from_str("_")),
+            (TimeRange.from_str("()"), TimeRange.from_str("_"),
+             TimeRange.from_str("_")),
+            (TimeRange.from_str("_10:0)"), TimeRange.from_str("[0:0_"),
+             TimeRange.from_str("_")),
+            (TimeRange.from_str("[0:0_10:0)"), TimeRange.from_str("[5:0_"),
+             TimeRange.from_str("[0:0_")),
+            (TimeRange.from_str("[0:0_10:0)"), TimeRange.from_str("[5:0_15:0)"),
+             TimeRange.from_str("[0:0_15:0)")),
+            (TimeRange.from_str("[0:0_10:0)"), TimeRange.from_str("[10:0_15:0)"),
+             TimeRange.from_str("[0:0_15:0)")),
+            (TimeRange.from_str("()"), TimeRange.from_str("[5:0_"),
+             TimeRange.from_str("[5:0_")),
+            (TimeRange.from_str("()"), TimeRange.from_str("[5:0_15:0)"),
+             TimeRange.from_str("[5:0_15:0)")),
+            (TimeRange.from_str("()"), TimeRange.from_str("_15:0)"),
+             TimeRange.from_str("_15:0)")),
+        ]
+
+        for (first, second, expected) in test_data:
+            with self.subTest(first=first, second=second, expected=expected):
+                self.assertEqual(first.union_with_timerange(second), expected)
+
+        test_data = [
+            (TimeRange.from_str("_0:0)"), TimeRange.from_str("(0:0_")),
+            (TimeRange.from_str("[0:0_5:0)"), TimeRange.from_str("[10:0_15:0)")),
+        ]
+
+        for (first, second) in test_data:
+            with self.subTest(first=first, second=second):
+                with self.assertRaises(ValueError):
+                    first.union_with_timerange(second)