Bug fix one element series truncate #35547
Conversation
| @@ -9397,7 +9397,7 @@ def truncate( | |||
| if before > after: | |||
| raise ValueError(f"Truncate: {after} must be after {before}") | |||
|
|
|||
| if ax.is_monotonic_decreasing: | |||
| if len(ax) > 1 and ax.is_monotonic_decreasing: | |||
There was a problem hiding this comment.
Is there a reason why a single element container should be monotonic_decrasing? Wondering if the real issue isn't that property
There was a problem hiding this comment.
Both .is_monotonic_decreasing and .is_monotonic_increasing will return true for a single-element series. I don't think checking whether a single element series is "increasing" or "decreasing" really makes sense, so this fix really just prevents the code from checking.
The fix was suggested by @TomAugspurger , and the test is passing after the fix.
There was a problem hiding this comment.
I don't think checking whether a single element series is "increasing" or "decreasing" really makes sense
Yea that's my point. Instead of special casing this to check the length if the property returned False you would get the same result
There was a problem hiding this comment.
OK, misunderstood your point. I'll take a look at that code.
There was a problem hiding this comment.
I think length-1 (and probably length-0) sequences are both monotonically increasing and decreasing by definition.
There was a problem hiding this comment.
I think length-1 (and probably length-0) sequences are both monotonically increasing and decreasing by definition.
After digging a bit, I think you're right, though couldn't find a really good reference... Do you maybe have one?
doc/source/whatsnew/v1.1.1.rst
Outdated
| @@ -49,7 +49,7 @@ Categorical | |||
|
|
|||
| **Indexing** | |||
|
|
|||
| - | |||
| - Bug in series.truncate when trying to truncate a single-element series (:issue:`35544`) | |||
There was a problem hiding this comment.
should be :meth:`Series.truncate`
There was a problem hiding this comment.
Thanks for the review. This has been updated.
| before = pd.Timestamp("2020-08-02") | ||
| after = pd.Timestamp("2020-08-04") | ||
|
|
||
| assert series.truncate(before=before, after=after).equals(series) |
There was a problem hiding this comment.
can you use
result =
expected =
tm.assert_series_equal(result, expected)
There was a problem hiding this comment.
I updated this one too. The input and expected series are the same. I added a comment to clarify this.
jreback
added
Datetime
|
ok looks good, can you merge master and ping on green. |
…nt-series-truncate
@jreback All green! |
|
thanks @gabicca |
Co-authored-by: gabicca <[email protected]>
Awesome, Thanks! |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffFix when trying to truncate a one-element series within a correct date range returned an empty series instead of the originally passed-in series.