github-actions[bot] commented on code in PR #41136:
URL: https://github.com/apache/beam/pull/41136#discussion_r4517361605
##########
sdks/python/apache_beam/transforms/window.py:
##########
@@ -256,16 +256,24 @@ class IntervalWindow(windowed_value._IntervalWindowBase,
BoundedWindow):
end: End of window as seconds since Unix epoch.
"""
def __lt__(self, other):
+ if isinstance(other, IntervalWindow):
+ if self._end_micros != other._end_micros:
+ return self._end_micros < other._end_micros
+ return hash(self) < hash(other)
if self.end != other.end:
return self.end < other.end
return hash(self) < hash(other)
def intersects(self, other: 'IntervalWindow') -> bool:
- return other.start < self.end or self.start < other.end
+ return (
+ other._start_micros < self._end_micros or
+ self._start_micros < other._end_micros)
def union(self, other: 'IntervalWindow') -> 'IntervalWindow':
- return IntervalWindow(
- min(self.start, other.start), max(self.end, other.end))
+ w = IntervalWindow(None, None)
Review Comment:
**🟢 Low**: `IntervalWindow.intersects`, `union`, and `<` do not appear to
have direct unit tests in `window_test.py`. Consider adding a small unit test
exercising ` < `, `intersects()`, and `union()` (verifying `.start` and `.end`
on the unioned window) to ensure both pure-Python and Cython-compiled paths are
covered in CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
