Subtract: Remove the intersection of two ranges #36
Conversation
datetimerange/__init__.py
Outdated
| :Output: | ||
| .. parsed-literal:: | ||
|
|
||
| [2015-03-22T10:00:00+0900 - 2015-03-22T05:00+0900] |
There was a problem hiding this comment.
This should be [2015-03-22T10:00:00+0900 - 2015-03-22T10:05:00+0900]
| class TestDateTimeRange_subtract: | ||
| @pytest.mark.parametrize( | ||
| ["lhs", "rhs", "expected"], | ||
| [ |
There was a problem hiding this comment.
Maybe add a test case for when the lhs and rhs do not overlap?
There was a problem hiding this comment.
Addressed. See third test case.
| :Output: | ||
| .. parsed-literal:: | ||
|
|
||
| [2015-03-22T10:00:00+0900 - 2015-03-22T10:05:00+0900] |
There was a problem hiding this comment.
@mattasaminew addressed
| time_range.subtract(x) | ||
|
|
||
| :Output: | ||
| :: |
There was a problem hiding this comment.
A blank line is required after here to properly rendered as a reStructuredText.
| @@ -0,0 +1,15 @@ | |||
| Make an intersected time range | |||
There was a problem hiding this comment.
Is this titled as you intended?
test/test_dtr.py
Outdated
| ) | ||
| def test_exception(self, lhs, rhs, expected): | ||
| with pytest.raises(expected): | ||
| lhs.is_intersection(rhs) |
There was a problem hiding this comment.
Probably, subtract method should be called here.
| start_time_format=self.start_time_format, end_time_format=self.end_time_format), | ||
| DateTimeRange(start_datetime=overlap.end_datetime, end_datetime=self.end_datetime, | ||
| start_time_format=self.start_time_format, end_time_format=self.end_time_format) | ||
| ] |
There was a problem hiding this comment.
In the current implementation, a DateTimeRange split in two if subtract a DateTimeRange with a range that start-time equals end-time.
For example:
>>> from datetimerange import DateTimeRange
>>> time_range = DateTimeRange("2015-01-22T09:50:00+0900", "2015-01-22T10:00:00+0900")
>>> x = DateTimeRange("2015-01-22T09:55:00+0900", "2015-01-22T09:55:00+0900")
>>> time_range.subtract(x)
[2015-01-22T09:50:00+0900 - 2015-01-22T09:55:00+0900, 2015-01-22T09:55:00+0900 - 2015-01-22T10:00:00+0900]In this case, returning [self.copy()] might be more preferable.
What do you think about this behavior?
There was a problem hiding this comment.
Ah I see. It's a zero length range. I would say yeah it's not really a valid case for breaking up the range and it should not cut it into two.
datetimerange/__init__.py
Outdated
| from datetimerange import DateTimeRange | ||
| dtr0 = DateTimeRange("2015-03-22T10:00:00+0900", "2015-03-22T10:10:00+0900") | ||
| dtr1 = DateTimeRange("2015-03-22T10:05:00+0900", "2015-03-22T10:15:00+0900") | ||
| dtr0.subtract(dtr1) |
There was a problem hiding this comment.
The above four lines need an extra indentation to be properly displayed as reStructuredText.
datetimerange/__init__.py
Outdated
| The result will be [self.copy()] if the second range does not overlap the first | ||
| The result will be [] if the second range wholly encompasses the first range | ||
| The result will be [new_range] if the second range overlaps one end of the range | ||
| The result will be [new_range1, new_range2] if the second range is an internal sub range of the first |
There was a problem hiding this comment.
The above lines are a little bit hard to read in the HTML after the docs build.
How about modifying like the followings:
Remove a time range from this one and return the result.
- The result will be ``[self.copy()]`` if the second range does not overlap the first
- The result will be ``[]`` if the second range wholly encompasses the first range
- The result will be ``[new_range]`` if the second range overlaps one end of the range
- The result will be ``[new_range1, new_range2]`` if the second range is an internal sub range of the first
|
Please apply code formatter. You can do that as follows: |
Subtracting two ranges will result in an array of ranges: empty set -- the second range is equal or greater than the first single set -- the second set overlaps the beginning or end of the first or does not intersect at all two ranges -- the second set is within the first and slices it into two new pieces
| [ | ||
| DateTimeRange("2015-01-22T09:50:00+0900", "2015-01-22T10:30:00+0900"), | ||
| DateTimeRange("2015-01-22T09:55:00+0900", "2015-01-22T09:55:00+0900"), | ||
| [DateTimeRange("2015-01-22T09:50:00+0900", "2015-01-22T10:30:00+0900")], |
There was a problem hiding this comment.
@thombashi test case to cover that case.
| """ | ||
| overlap = self.intersection(x) | ||
| # No intersection, return a copy of the original | ||
| if not overlap.is_set() or overlap.get_timedelta_second() <= 0: |
There was a problem hiding this comment.
@thombashi covers the case where you attempt to subtract a zero length range.
|
@thombashi looks like the test failures are due to python 3.5. This is all passing locally. |
I'll fix CI. |
Subtracting two ranges will result in an array of ranges:
empty set -- the second range is equal or greater than the first
single set -- the second set overlaps the beginning or end of the first or does not intersect at all
two ranges -- the second set is within the first and slices it into two new pieces