Skip to content

Conversation

@zhengruifeng
Copy link
Contributor

What changes were proposed in this pull request?

head should return self.limit(n).toPandas() instead of self.toPandas()

Why are the changes needed?

bugfix

Does this PR introduce any user-facing change?

No

How was this patch tested?

existing tests

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Please file a JIRA issue and use in the PR title, @zhengruifeng .

@zhengruifeng zhengruifeng changed the title [CONNECT][PYTHON] Fix Dataframe.head [SPARK-40953][CONNECT][PYTHON] Fix Dataframe.head Oct 28, 2022
@zhengruifeng
Copy link
Contributor Author

@dongjoon-hyun thanks for the reminder😄

Copy link
Contributor

@itholic itholic left a comment

Choose a reason for hiding this comment

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

LGTM

@HyukjinKwon HyukjinKwon changed the title [SPARK-40953][CONNECT][PYTHON] Fix Dataframe.head [SPARK-40953][CONNECT][PYTHON] Fix DataFrame.head to collect specified number of rows Oct 30, 2022
@HyukjinKwon
Copy link
Member

Merged to master.

@zhengruifeng zhengruifeng deleted the connect_head_fix branch October 31, 2022 02:21
@amaliujia
Copy link
Contributor

nice catch!

HyukjinKwon pushed a commit that referenced this pull request Oct 31, 2022
…head(n)

### What changes were proposed in this pull request?

Following up on #38424, we can actually add a test case for `head(n)`.

### Why are the changes needed?

Improve test coverage.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

UT

Closes #38442 from amaliujia/follow-up-head-n.

Authored-by: Rui Wang <rui.wang@databricks.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
…ied number of rows

### What changes were proposed in this pull request?
`head` should return `self.limit(n).toPandas()` instead of `self.toPandas()`

### Why are the changes needed?
bugfix

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
existing tests

Closes apache#38424 from zhengruifeng/connect_head_fix.

Authored-by: Ruifeng Zheng <ruifengz@apache.org>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
…head(n)

### What changes were proposed in this pull request?

Following up on apache#38424, we can actually add a test case for `head(n)`.

### Why are the changes needed?

Improve test coverage.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

UT

Closes apache#38442 from amaliujia/follow-up-head-n.

Authored-by: Rui Wang <rui.wang@databricks.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants