Skip to content

Bug fix one element series truncate #35547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 7, 2020
Merged

Bug fix one element series truncate #35547

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f91dd0
BUG: series.truncate fix for one-element series (#35544)
gabicca Aug 4, 2020
a72a62b
BUG: series.truncate fix for one-element series (#35544) - black form…
gabicca Aug 4, 2020
f667212
BUG: series.truncate fix for one-element series (#35544) - updated re…
gabicca Aug 4, 2020
2e85aef
BUG: series.truncate fix for one-element series (#35544) - PR review …
gabicca Aug 5, 2020
215e65d
Merge remote-tracking branch 'upstream/master' into bug-fix-one-eleme…
gabicca Aug 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v1.1.1.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Categorical

**Indexing**

-
- Bug in series.truncate when trying to truncate a single-element series (:issue:`35544`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be :meth:`Series.truncate`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. This has been updated.


.. ---------------------------------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion pandas/core/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why a single element container should be monotonic_decrasing? Wondering if the real issue isn't that property

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, misunderstood your point. I'll take a look at that code.

Copy link
Contributor

@TomAugspurger TomAugspurger Aug 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think length-1 (and probably length-0) sequences are both monotonically increasing and decreasing by definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

before, after = after, before

slicer = [slice(None, None)] * self._AXIS_LEN
Expand Down
8 changes: 8 additions & 0 deletions pandas/tests/series/methods/test_truncate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,11 @@ def test_truncate_multiindex(self):
expected = df.col

tm.assert_series_equal(result, expected)

def test_truncate_one_element_series(self):
# GH 35544
series = pd.Series([0.1], index=pd.DatetimeIndex(["2020-08-04"]))
before = pd.Timestamp("2020-08-02")
after = pd.Timestamp("2020-08-04")

assert series.truncate(before=before, after=after).equals(series)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you use

result = 
expected = 
tm.assert_series_equal(result, expected)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this one too. The input and expected series are the same. I added a comment to clarify this.