Skip to content

[SPARK-22700][ML] Bucketizer.transform incorrectly drops row containing NaN#19894

Closed
zhengruifeng wants to merge 2 commits into
apache:masterfrom
zhengruifeng:bucketizer_nan
Closed

[SPARK-22700][ML] Bucketizer.transform incorrectly drops row containing NaN#19894
zhengruifeng wants to merge 2 commits into
apache:masterfrom
zhengruifeng:bucketizer_nan

Conversation

@zhengruifeng

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

only drops the rows containing NaN in the input columns

How was this patch tested?

existing tests and added tests

@SparkQA

SparkQA commented Dec 5, 2017

Copy link
Copy Markdown

Test build #84481 has finished for PR 19894 at commit 01604c7.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA

SparkQA commented Dec 5, 2017

Copy link
Copy Markdown

Test build #84485 has finished for PR 19894 at commit eaebedb.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@zhengruifeng

Copy link
Copy Markdown
Contributor Author

ping @MLnick ?

@hhbyyh

hhbyyh commented Dec 8, 2017

Copy link
Copy Markdown
Contributor

LGTM. Good fix.

@WeichenXu123 WeichenXu123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@MLnick

MLnick commented Dec 13, 2017

Copy link
Copy Markdown
Contributor

LGTM thanks! Merged to master

@asfgit asfgit closed this in 8743509 Dec 13, 2017
@zhengruifeng zhengruifeng deleted the bucketizer_nan branch December 13, 2017 07:13
@jkbradley

jkbradley commented Feb 8, 2018

Copy link
Copy Markdown
Member

I'm going to backport this to 2.2 since it's a correctness bug.
Update: Actually, it doesn't backport cleanly. I can do a backport PR in a few days, or feel free to go ahead and send one. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants