Skip to content
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.

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

head(0) and tail(0) return empty DataFrames #11937

Merged
merged 1 commit into from
Jan 2, 2016
Merged

Conversation

trvrm
Copy link

@trvrm trvrm commented Dec 31, 2015

closes #11930

frame.head(0) and frame.tail(0) now return empty DataFrames.

return self
if n == 0:
return self.iloc[l:]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think self.iloc[0:0] is clearer here. I don't think you need l anymore (was it ever needed?).

The side question here is: should we check that n>0? I don't think we need to... as this will make it kinda useful: it'll return the rest of the dataframe except the head(n) == tail(len(df) - n).

Edit: or n>=0.

@jreback
Copy link
Contributor

jreback commented Dec 31, 2015

make sure to test groupby! (need updating maybe)

@trvrm
Copy link
Author

trvrm commented Dec 31, 2015

Fixing the broken unit test....

@@ -2146,8 +2147,10 @@ def tail(self, n=5):
Returns last n rows
"""
l = len(self)
if l == 0 or n == 0:
if l == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

This l is not required. self.iloc[-0:] == self :)

Copy link
Author

Choose a reason for hiding this comment

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

So why was it there in the first place? I don't really understand what its doing in either head() or tail().

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it was due to the n == 0 or n == l check that was there before. Even in that case there's no need to avoid the iloc in the latter case; code is simpler without it.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I've taken the checks out.

@hayd
Copy link
Contributor

hayd commented Dec 31, 2015

@trvrm
Copy link
Author

trvrm commented Dec 31, 2015

I don't really understand why Travis is failing - it seems to be some kind of connection error? Is that something I've done wrong?

@hayd
Copy link
Contributor

hayd commented Dec 31, 2015

Looks unrelated. squash and maybe the tests will pass next time :)

@trvrm
Copy link
Author

trvrm commented Jan 1, 2016

I'm sorry, I'm not sure what you mean by 'squash'.

@jreback
Copy link
Contributor

jreback commented Jan 1, 2016

@trvrm
Copy link
Author

trvrm commented Jan 1, 2016

Ah, ok. I think I did that right, hopefully the tests will pass this time.

@TomAugspurger
Copy link
Contributor

Need more thing, a release note in doc/source/whatsnew/v0.18.0, maybe under the API changes section, to document the change. You can squash that commit as well (which you did correctly). Thanks!

@trvrm
Copy link
Author

trvrm commented Jan 1, 2016

Added a note in the release docs.

@jreback jreback added Reshaping Concat, Merge/Join, Stack/Unstack, Explode API Design labels Jan 2, 2016
@jreback jreback added this to the 0.18.0 milestone Jan 2, 2016
@@ -166,6 +166,7 @@ Backwards incompatible API changes

- The parameter ``out`` has been removed from the ``Series.round()`` method. (:issue:`11763`)
- ``DataFrame.round()`` leaves non-numeric columns unchanged in its return, rather than raises. (:issue:`11885`)
- ``DataFrame.head(0)`` and ``DataFrame.tail(0)`` return empty frames, rather than ``self``. (:issue:`11937`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention Series as well here

Copy link
Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

actually Series was already doing this. But that's fine.

unit test for Series.head(0), Series.tail(0)

fixed breaking unit tests on Series.head(0)/Series.tail(0)

removed unnecessary length checks

added documentation

mention series in API docs
@jreback
Copy link
Contributor

jreback commented Jan 2, 2016

ok, lgtm. ping when green.

hayd added a commit that referenced this pull request Jan 2, 2016
head(0) and tail(0) return empty DataFrames
@hayd hayd merged commit a1e7d53 into pandas-dev:master Jan 2, 2016
@hayd
Copy link
Contributor

hayd commented Jan 2, 2016

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataFrame.head(0) doesn't return an empty frame
4 participants