Skip to content

Added Documentation to specify that DataFrame.last() needs the index to be sorted to deliver the expected results #38536

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
Dec 17, 2020
Merged

Added Documentation to specify that DataFrame.last() needs the index to be sorted to deliver the expected results #38536

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
60e563f
Update generic.py
aflah02 Dec 17, 2020
9f2cb04
Update generic.py
aflah02 Dec 17, 2020
4c2ba8d
Update generic.py
aflah02 Dec 17, 2020
a12a2da
Update generic.py
aflah02 Dec 17, 2020
67a7771
Update generic.py
aflah02 Dec 17, 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
3 changes: 3 additions & 0 deletions pandas/core/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8442,6 +8442,9 @@ def last(self: FrameOrSeries, offset) -> FrameOrSeries:
When having a DataFrame with dates as index, this function can
select the last few rows based on a date offset.

Note that for the last() method to work, the DatetimeIndex
Copy link
Member

Choose a reason for hiding this comment

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

Personal opinion but I'd rather rephrase the paragraph above to make this clear:

For a DataFrame with a sorted DatetimeIndex, this function selects the last few rows based on a date offset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this seems to be better, I wasn't thinking of changing the original section initially but if that's okay then I'll change that too thanks :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arw2019 I did make the changes but I was wondering as was commented in the issue earlier as well do we need to raise an error message as well incase the index is not sorted?

Copy link
Member

@arw2019 arw2019 Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@arw2019 I did make the changes but I was wondering as was commented in the issue earlier as well do we need to raise an error message as well incase the index is not sorted?

It uses binary search under the hood (logarithmic time) so, given no extra information, vertifying sortedness (linear time) would degrade performance. If sortedness information is stored somehow then I agree we should throw but AFAIK it isn't in general for a DatetimeIndex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohkk understood thanks 👍

must be sorted otherwise it will not work as intended.

Parameters
----------
offset : str, DateOffset, dateutil.relativedelta
Expand Down