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

[SPARK-41201][CONNECT][PYTHON][TEST][FOLLOWUP] Reenable test_fill_na #38763

Closed

Conversation

zhengruifeng
Copy link
Contributor

What changes were proposed in this pull request?

Reenable test_fill_na

Why are the changes needed?

test_fill_na was disabled by mistake in #38723

Does this PR introduce any user-facing change?

no

How was this patch tested?

reenabled test

@amaliujia
Copy link
Contributor

amaliujia commented Nov 23, 2022

LGTM

If you are interested in, can you BTW follow up in this PR on

Basically I think

        self.assert_eq(
            cdf.drop(cdf.a, cdf.x).toPandas(),
            sdf.drop("a", "x").toPandas(),
        )

these two case are not equivalent?

They should be

        self.assert_eq(
            cdf.drop(cdf.a, cdf.x).toPandas(),
            sdf.drop(cdf.a, cdf.x).toPandas(),
        )

?

@zhengruifeng
Copy link
Contributor Author

@amaliujia that is on purpose, sdf.x will just throw an exception since sdf don't contains x column, but in connect df cdf , cdf.x will not throw an exception since it will not check the schema.

@amaliujia
Copy link
Contributor

@zhengruifeng thanks for the clarification!

@zhengruifeng
Copy link
Contributor Author

merged into master

@zhengruifeng zhengruifeng deleted the connect_reenable_test_fillna branch November 23, 2022 03:35
beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 15, 2022
### What changes were proposed in this pull request?
Reenable test_fill_na

### Why are the changes needed?
`test_fill_na` was disabled by mistake in apache#38723

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

### How was this patch tested?
reenabled test

Closes apache#38763 from zhengruifeng/connect_reenable_test_fillna.

Authored-by: Ruifeng Zheng <ruifengz@apache.org>
Signed-off-by: Ruifeng Zheng <ruifengz@apache.org>
beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 18, 2022
### What changes were proposed in this pull request?
Reenable test_fill_na

### Why are the changes needed?
`test_fill_na` was disabled by mistake in apache#38723

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

### How was this patch tested?
reenabled test

Closes apache#38763 from zhengruifeng/connect_reenable_test_fillna.

Authored-by: Ruifeng Zheng <ruifengz@apache.org>
Signed-off-by: Ruifeng Zheng <ruifengz@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants