Skip to content

[Spark-8530] [ML] add python API for MinMaxScaler #7150

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

Closed
wants to merge 7 commits into from

Conversation

hhbyyh
Copy link
Contributor

@hhbyyh hhbyyh commented Jul 1, 2015

jira: https://issues.apache.org/jira/browse/SPARK-8530

add python API for MinMaxScaler
jira for MinMaxScaler: https://issues.apache.org/jira/browse/SPARK-7514

@SparkQA
Copy link

SparkQA commented Jul 1, 2015

Test build #36235 has finished for PR 7150 at commit 77f57ef.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class Heartbeat(workerId: String, worker: RpcEndpointRef) extends DeployMessage
    • case class RegisteredWorker(master: RpcEndpointRef, masterWebUiUrl: String) extends DeployMessage
    • case class RegisterApplication(appDescription: ApplicationDescription, driver: RpcEndpointRef)
    • case class RegisteredApplication(appId: String, master: RpcEndpointRef) extends DeployMessage
    • case class SubmitDriverResponse(
    • case class KillDriverResponse(
    • case class MasterChanged(master: RpcEndpointRef, masterWebUiUrl: String)
    • class MinMaxScaler(JavaEstimator, HasInputCol, HasOutputCol):
    • class MinMaxScalerModel(JavaModel):

@jkbradley
Copy link
Member

reviewing now

@@ -24,7 +24,7 @@
__all__ = ['Binarizer', 'HashingTF', 'IDF', 'IDFModel', 'NGram', 'Normalizer', 'OneHotEncoder',
'PolynomialExpansion', 'RegexTokenizer', 'StandardScaler', 'StandardScalerModel',
'StringIndexer', 'StringIndexerModel', 'Tokenizer', 'VectorAssembler', 'VectorIndexer',
'Word2Vec', 'Word2VecModel']
'Word2Vec', 'Word2VecModel', 'MinMaxScaler', 'MinMaxScalerModel']
Copy link
Member

Choose a reason for hiding this comment

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

Please keep sorted

@jkbradley
Copy link
Member

This looks good, except for those tiny items & merge conflicts.

@jkbradley
Copy link
Member

Hm, the generated doc also looks odd b/c of the Latex not being render as Latex. Does the math markup element work? [http://sphinx-doc.org/ext/math.html]
screen shot 2015-08-13 at 10 48 21 am

@SparkQA
Copy link

SparkQA commented Aug 19, 2015

Test build #41206 has finished for PR 7150 at commit 7b97e6a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class MinMaxScaler(JavaEstimator, HasInputCol, HasOutputCol):
    • class MinMaxScalerModel(JavaModel):

@hhbyyh
Copy link
Contributor Author

hhbyyh commented Aug 19, 2015

@jkbradley Sorry for the late update. I got some internet issues and cannot install Sphinx for now, so I simplified the comments. Let me know if it's not clear enough.

@inherit_doc
class MinMaxScaler(JavaEstimator, HasInputCol, HasOutputCol):
"""
Rescale each feature individually to a common range [min, max] linearly using column summary
Copy link
Member

Choose a reason for hiding this comment

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

mark with .. note:: Experimental

@jkbradley
Copy link
Member

@hhbyyh Can you please fix the merge conflicts? I have time to get this merged now. Thanks!

@SparkQA
Copy link

SparkQA commented Sep 10, 2015

Test build #42237 has finished for PR 7150 at commit 9785b56.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class MinMaxScaler(JavaEstimator, HasInputCol, HasOutputCol):
    • class MinMaxScalerModel(JavaModel):
    • class VectorSlicer(JavaTransformer, HasInputCol, HasOutputCol):

@hhbyyh
Copy link
Contributor Author

hhbyyh commented Sep 11, 2015

@jkbradley Updated according to the comments. Thanks for helping review.

@jkbradley
Copy link
Member

LGTM. Merging with master. Thanks!

@asfgit asfgit closed this in 5f46444 Sep 11, 2015
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.

3 participants