-
Notifications
You must be signed in to change notification settings - Fork 330
Add Param and methods to Spark.ML #586
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
Conversation
|
@suhsteve / @Niharikadutta Can you help with review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few nit comments, but generally LGTM
src/csharp/Microsoft.Spark.E2ETest/IpcTests/ML/Feature/BucketizerTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nits, but otherwise LGTM. Thanks @GoEddie !
src/csharp/Microsoft.Spark.E2ETest/IpcTests/ML/Param/ParamTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Steve Suh <suhsteve@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @GoEddie!
This adds the Param class and the appropriate methods on FeatureBase to be shared by all the Features that implement
https://spark.apache.org/docs/1.5.1/api/java/org/apache/spark/ml/param/Params.html.The scala class is
org.apache.spark.ml.param.Parambut the .NET naming standard isSpark.ML.Param.Paramso defining it in C# means usingParam.Param- not sure if that is acceptable or if it is worth moving it up a level in the namespace?