Skip to content

Support for Bucketizer #378

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

Merged
merged 27 commits into from
Jan 17, 2020
Merged

Support for Bucketizer #378

merged 27 commits into from
Jan 17, 2020

Conversation

GoEddie
Copy link
Contributor

@GoEddie GoEddie commented Dec 29, 2019

This implements Bucketizer (https://spark.apache.org/docs/latest/ml-features#bucketizer) and completes #313

This implements the concrete methods on Bucketizer that are needed to run the transform.

@GoEddie GoEddie changed the title WIP: Support for Bucketizer Support for Bucketizer Dec 29, 2019
@imback82 imback82 added the enhancement New feature or request label Dec 30, 2019
@imback82
Copy link
Contributor

imback82 commented Jan 8, 2020

@suhsteve can you help reviewing this?

@GoEddie
Copy link
Contributor Author

GoEddie commented Jan 8, 2020

I have updated after the comments, let me know whether the DoubleArrayArrayParam is ok or not - if you would prefer that we support double[][] on the SerDer let me know.

@GoEddie GoEddie changed the title Support for Bucketizer WIP: Support for Bucketizer Jan 13, 2020
@GoEddie GoEddie changed the title WIP: Support for Bucketizer Support for Bucketizer Jan 13, 2020
@GoEddie
Copy link
Contributor Author

GoEddie commented Jan 13, 2020

@suhsteve I have re-written this now so that SerDe can understand a double[][] - I used a 'A' as the identifier, if there are any rules about which letter to use let me know and i'll change it.

@GoEddie GoEddie changed the title Support for Bucketizer [WIP]: Support for Bucketizer Jan 15, 2020
@GoEddie GoEddie changed the title [WIP]: Support for Bucketizer Support for Bucketizer Jan 15, 2020
@GoEddie
Copy link
Contributor Author

GoEddie commented Jan 15, 2020

I've made the changes after the code review, let me know any other changes

suhsteve
suhsteve previously approved these changes Jan 15, 2020
Copy link
Member

@suhsteve suhsteve left a comment

Choose a reason for hiding this comment

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

One minor change. LGTM! Thanks @GoEddie

@GoEddie
Copy link
Contributor Author

GoEddie commented Jan 15, 2020

Great thanks :)

Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

I have minor comments, but generally looks good to me.

Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @GoEddie!

@imback82 imback82 merged commit 739688e into dotnet:master Jan 17, 2020
@GoEddie GoEddie deleted the bucketizer-ml-313 branch March 5, 2020 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants