Skip to content

Conversation

@mumuhhh
Copy link
Contributor

@mumuhhh mumuhhh commented May 10, 2022

What is the purpose of the change

  • Fix finding buckets error in Bucketizer

Brief change log

  • Changed inputCols to splits in Bucketizer and updated the corresponding unit test.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @public(Evolving): (no)
  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (Java doc)

@zhipeng93
Copy link
Contributor

Thanks for the fix @mumuhhh ! LGTM.

Two minor comments:

  • Could you add a unit test that the number of inputCols is not the same as number splits?
  • Could you re-polish the commit message, such as [hotfix] Fix finding buckets error in Bucketizer?

@mumuhhh mumuhhh changed the title [hotfix] Bucketizer split point search [hotfix] Fix finding buckets error in Bucketizer May 10, 2022
@mumuhhh mumuhhh force-pushed the patch-2 branch 2 times, most recently from 2cf359a to 372b82e Compare May 10, 2022 06:50
@zhipeng93
Copy link
Contributor

Thanks for the fix @mumuhhh ! Merged :)

@zhipeng93 zhipeng93 merged commit f6b071f into apache:master May 10, 2022
@zhipeng93 zhipeng93 self-assigned this May 10, 2022
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.

2 participants