You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mengxr <gi...@git.apache.org> on 2015/02/24 01:11:43 UTC

[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

GitHub user mengxr opened a pull request:

    https://github.com/apache/spark/pull/4735

    [SPARK-5886][ML] Add label indexer

    This PR adds label indexer, which takes a column of labels and outputs an integer column with labels indexed by their frequency.
    
    TODOs:
    - [ ] store feature to index map in output metadata

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mengxr/spark SPARK-5886

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/4735.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #4735
    
----
commit e7f41cb6f6754717397d1df958bc556953040760
Author: Xiangrui Meng <me...@databricks.com>
Date:   2015-02-24T00:01:58Z

    add labelIndexer with tests

commit d0d45f92f369cc2f608e15ebc4e90233ec3f901e
Author: Xiangrui Meng <me...@databricks.com>
Date:   2015-02-24T00:07:45Z

    update doc

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85841449
  
      [Test build #29149 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29149/consoleFull) for   PR 4735 at commit [`457166e`](https://github.com/apache/spark/commit/457166eb74eb5bd3d7d642a3a13f2423f216cc9a).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27168622
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  // TODO: handle unseen labels
    +
    +  override def fit(dataset: DataFrame, paramMap: ParamMap): LabelIndexerModel = {
    +    val map = this.paramMap ++ paramMap
    +    val counts = dataset.select(map(labelCol)).map(_.getString(0)).countByValue()
    +    val labels = counts.toSeq.sortBy(-_._2).map(_._1).toArray
    --- End diff --
    
    Hm because the most common label is encoded as 0.0? it feels like a funny optimization since it's not "really" a 0, and these values are handled as individual values or dense vectors mostly (right?). Is counting the whole data set by label really that cheap? it still means looking at every datum even if there are few labels. I don't strongly object or anything, this just hadn't occurred to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27508064
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    --- End diff --
    
    I agree with this since we have used "label" to mean label for prediction thus far.  (But @mengxr disagreed based on precedence from sklearn; you can see the comments above.)
    
    I also added questions in the JIRA about whether this class should support multiple input and output columns and support more native types than String.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27145624
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  // TODO: handle unseen labels
    +
    +  override def fit(dataset: DataFrame, paramMap: ParamMap): LabelIndexerModel = {
    +    val map = this.paramMap ++ paramMap
    +    val counts = dataset.select(map(labelCol)).map(_.getString(0)).countByValue()
    +    val labels = counts.toSeq.sortBy(-_._2).map(_._1).toArray
    --- End diff --
    
    Instead of ordering by count, which requires all that counting, can labels just be ordered alphabetically? maybe I miss why it's important to sort them this way, but I'd think it's arbitrary. A deterministic function of the labels themselves might be less surprising or something later, though I can't name a specific problem it would solve, now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85744647
  
      [Test build #29115 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29115/consoleFull) for   PR 4735 at commit [`f8b30f4`](https://github.com/apache/spark/commit/f8b30f45047315efc13bba8fa7084402a353680c).
     * This patch **passes all tests**.
    
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase `
      * `case class Alias(child: Expression, name: String)(`
      * `class Column(protected[sql] val expr: Expression) extends Logging `



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85747326
  
      [Test build #29127 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29127/consoleFull) for   PR 4735 at commit [`457166e`](https://github.com/apache/spark/commit/457166eb74eb5bd3d7d642a3a13f2423f216cc9a).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27167452
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  // TODO: handle unseen labels
    +
    +  override def fit(dataset: DataFrame, paramMap: ParamMap): LabelIndexerModel = {
    +    val map = this.paramMap ++ paramMap
    +    val counts = dataset.select(map(labelCol)).map(_.getString(0)).countByValue()
    +    val labels = counts.toSeq.sortBy(-_._2).map(_._1).toArray
    --- End diff --
    
    Since we need to distinct labels anyway, there is little overhead with count and sort. Couple benefits:
    
    1. better sparsity as there will be more zeros
    2. easy inspection as common labels are at the beginning.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85769350
  
      [Test build #29127 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29127/consoleFull) for   PR 4735 at commit [`457166e`](https://github.com/apache/spark/commit/457166eb74eb5bd3d7d642a3a13f2423f216cc9a).
     * This patch **fails Spark unit tests**.
    
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase `
      * `    implicit class DslSymbol(sym: Symbol) extends ImplicitAttribute `



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add StringIndexer as a featur...

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-92220115
  
    Merged into master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-75869252
  
    Is LabelIndexer going to be different from FeatureIndexer.  We will need a transformer to index features as well.  I've been planning to revive my old PR for DatasetIndexer which was headed this way: [https://github.com/apache/spark/pull/3000], but it would be good not to duplicate efforts.
    
    If you want to merge the 2, that would be great, or I could push an update.  The main thing I liked about DatasetIndexer is that it could be used to choose which features to treat as continuous vs. categorical based on a maxCategories threshold.  Choosing automatically helps users to avoid having to hand-pick columns as categorical vs. continuous.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27145685
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  // TODO: handle unseen labels
    +
    +  override def fit(dataset: DataFrame, paramMap: ParamMap): LabelIndexerModel = {
    +    val map = this.paramMap ++ paramMap
    +    val counts = dataset.select(map(labelCol)).map(_.getString(0)).countByValue()
    +    val labels = counts.toSeq.sortBy(-_._2).map(_._1).toArray
    +    val model = new LabelIndexerModel(this, map, labels)
    +    Params.inheritValues(map, this, model)
    +    model
    +  }
    +
    +  override def transformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    validateAndTransformSchema(schema, paramMap)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * Model fitted by [[LabelIndexer]].
    + */
    +@AlphaComponent
    +class LabelIndexerModel private[ml] (
    +    override val parent: LabelIndexer,
    +    override val fittingParamMap: ParamMap,
    +    labels: Array[String]) extends Model[LabelIndexerModel] with LabelIndexerBase {
    +
    +  private val labelToIndex: OpenHashMap[String, Double] = {
    +    val n = labels.length
    +    val map = new OpenHashMap[String, Double](n)
    +    var i = 0
    +    while (i < n) {
    +      map.update(labels(i), i)
    +      i += 1
    +    }
    +    map
    +  }
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  override def transform(dataset: DataFrame, paramMap: ParamMap): DataFrame = {
    +    val map = this.paramMap ++ paramMap
    +    val indexer = udf { label: String =>
    +      if (labelToIndex.contains(label)) {
    +        labelToIndex(label)
    +      } else {
    +        throw new SparkException(s"Unseen label: $label.")
    +      }
    +    }
    +    val outputColName = map(outputCol)
    +    val metadata = NominalAttribute.defaultAttr
    +      .withName(outputColName).withValues(labels).toStructField().metadata
    --- End diff --
    
    Yep so here is where the labels go into the metadata, in order. That's good.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27927802
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    --- End diff --
    
    For model persistence, we need to save labels to disk. String would be ideal and may be sufficient.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-75669242
  
      [Test build #27868 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27868/consoleFull) for   PR 4735 at commit [`d0d45f9`](https://github.com/apache/spark/commit/d0d45f92f369cc2f608e15ebc4e90233ec3f901e).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27535066
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    --- End diff --
    
    I agree


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r28124941
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  // TODO: handle unseen labels
    +
    +  override def fit(dataset: DataFrame, paramMap: ParamMap): LabelIndexerModel = {
    +    val map = this.paramMap ++ paramMap
    +    val counts = dataset.select(map(labelCol)).map(_.getString(0)).countByValue()
    +    val labels = counts.toSeq.sortBy(-_._2).map(_._1).toArray
    +    val model = new LabelIndexerModel(this, map, labels)
    +    Params.inheritValues(map, this, model)
    +    model
    +  }
    +
    +  override def transformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    validateAndTransformSchema(schema, paramMap)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * Model fitted by [[LabelIndexer]].
    + */
    +@AlphaComponent
    +class LabelIndexerModel private[ml] (
    +    override val parent: LabelIndexer,
    +    override val fittingParamMap: ParamMap,
    +    labels: Array[String]) extends Model[LabelIndexerModel] with LabelIndexerBase {
    +
    +  private val labelToIndex: OpenHashMap[String, Double] = {
    +    val n = labels.length
    +    val map = new OpenHashMap[String, Double](n)
    +    var i = 0
    +    while (i < n) {
    +      map.update(labels(i), i)
    +      i += 1
    +    }
    +    map
    +  }
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  override def transform(dataset: DataFrame, paramMap: ParamMap): DataFrame = {
    +    val map = this.paramMap ++ paramMap
    +    val indexer = udf { label: String =>
    +      if (labelToIndex.contains(label)) {
    +        labelToIndex(label)
    +      } else {
    +        throw new SparkException(s"Unseen label: $label.")
    --- End diff --
    
    I left a TODO for this. The issues are
    
    1. if we assign unseen label a unique index, it creases the number of values by 1 and a binary feature is no longer binary.
    2. if we map unseen label to an exiting index, it may silently change the behavior and users may not be aware of it.
    3. another option is to map the unseen label to `Double.NaN`, which is treated as missing value and can be filled in later.
    
    So I think we can discuss this in a follow-up PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85720964
  
    This PR contains changes in #5171.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-75681137
  
      [Test build #27868 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27868/consoleFull) for   PR 4735 at commit [`d0d45f9`](https://github.com/apache/spark/commit/d0d45f92f369cc2f608e15ebc4e90233ec3f901e).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase `



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27279180
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  // TODO: handle unseen labels
    +
    +  override def fit(dataset: DataFrame, paramMap: ParamMap): LabelIndexerModel = {
    +    val map = this.paramMap ++ paramMap
    +    val counts = dataset.select(map(labelCol)).map(_.getString(0)).countByValue()
    +    val labels = counts.toSeq.sortBy(-_._2).map(_._1).toArray
    --- End diff --
    
    In vector assembler, where we merge multiple columns into a vector column, the encoded indices are stored inside a sparse vector. For multiclass labels, this doesn't help much. But for binary labels, this saves at least half of the storage.
    
    `countByValue` costs about the same as `distinct`:
    https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/rdd/RDD.scala#L1041
    https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/rdd/RDD.scala#L313


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r28035557
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    --- End diff --
    
    Since the data are already in a DataFrame, we know we can save it to disk.  But keeping it simple with String is fine for now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85871624
  
      [Test build #29149 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29149/consoleFull) for   PR 4735 at commit [`457166e`](https://github.com/apache/spark/commit/457166eb74eb5bd3d7d642a3a13f2423f216cc9a).
     * This patch **passes all tests**.
    
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase `



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-75877168
  
    The `label` in the name is quite general. It could be labels used in classification or just an arbitrary column with string labels. This is the same as `LabelEncoder` in sklearn. The `LabelIndexer` should also create ML attributes using the labels.
    
    #3000 takes an `RDD[Vector]` and tries to decide which columns should be categorical. I don't see an overlap in terms of functionality. If the plan is to make `DatasetIndexer` a feature transformer that handles both `string labels -> indices` and `numeric -> categorical`, we can definitely go with this direction. Btw, having a single `maxCategories` might cause unexpected problems, e.g., treating categorical features as continuous. We may still need to ask users to specify which columns are categorical.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-91468355
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30015/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-91468332
  
      [Test build #30015 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30015/consoleFull) for   PR 4735 at commit [`700e70f`](https://github.com/apache/spark/commit/700e70fdf9b5fc649fce8be12a5bab23d0bc22c8).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class StringIndexer extends Estimator[StringIndexerModel] with StringIndexerBase `
      * `class SparseMatrix(Matrix):`
    
     * This patch does not change any dependencies.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85744658
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29115/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-75905836
  
    > LabelIndexer
    
    Sounds OK, though I wonder if some people will think it only applies to "labels" since we use that term for prediction.
    
    > String vs. numeric
    
    Oops, I didn't realize this was taking String.  (I had only looked at the JIRA and PR descriptions, not the code.)  They sound different.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by petro-rudenko <gi...@git.apache.org>.
Github user petro-rudenko commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27486767
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    --- End diff --
    
    If it suppose to be general indexer, not just for label column, maybe it makes sense to call it ColumnIndexer and use setInputCol instead.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r25285933
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,115 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.param._
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{IntegerType, StringType, StructField, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    // TODO: Store ML attributes in metadata.
    +    val outputFields = inputFields :+ StructField(map(outputCol), IntegerType, nullable = false)
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an integer column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * The most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  // TODO: Allow unseen labels in transform.
    +
    +  override def fit(dataset: DataFrame, paramMap: ParamMap): LabelIndexerModel = {
    +    val map = this.paramMap ++ paramMap
    +    val counts = dataset.select(map(labelCol)).map(_.getString(0)).countByValue()
    +    val labels = counts.toSeq.sortBy(-_._2).map(_._1).toArray
    +    val model = new LabelIndexerModel(this, map, labels)
    +    Params.inheritValues(map, this, model)
    +    model
    +  }
    +
    +  override def transformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    validateAndTransformSchema(schema, paramMap)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * Model fitted by [[LabelIndexer]].
    + */
    +@AlphaComponent
    +class LabelIndexerModel private[ml] (
    +    override val parent: LabelIndexer,
    +    override val fittingParamMap: ParamMap,
    +    labels: Array[String]) extends Model[LabelIndexerModel] with LabelIndexerBase {
    --- End diff --
    
    The `labels` in a fitted model should be viewed as immutable. If there are new labels, we can train a new `LabelIndexerModel`. We are gonna put label names into the metadata, so we still tracks which index maps to which label.
    
    We can add an option to `LabelIndexer` that allows users to specify part of the mapping. If we implement this option, then we can keep the original ordering unchanged on new datasets.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-91470977
  
      [Test build #30021 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30021/consoleFull) for   PR 4735 at commit [`d82575f`](https://github.com/apache/spark/commit/d82575fd9a29149ecd5979baa70091d567c3c5bd).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27760015
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    --- End diff --
    
    This will be shorter if you use the utility methods checkInputColumn and addOutputColumn.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-91491600
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30021/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-91450754
  
      [Test build #30015 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30015/consoleFull) for   PR 4735 at commit [`700e70f`](https://github.com/apache/spark/commit/700e70fdf9b5fc649fce8be12a5bab23d0bc22c8).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by petro-rudenko <gi...@git.apache.org>.
Github user petro-rudenko commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27510186
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    --- End diff --
    
    Yes, but in jira's example it uses setInputCol rather then setFeatureCol:
    ```scala
    val i = new LabelIndexer()
      .setInputCol("country")
      .setOutputCol("countryIndex")
    ```
    which makes more sense as for me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85841044
  
    test this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r28124748
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    --- End diff --
    
    I rename the class to `StringIndexer` and changed `labelCol` to `inputCol`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85722512
  
      [Test build #29115 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29115/consoleFull) for   PR 4735 at commit [`f8b30f4`](https://github.com/apache/spark/commit/f8b30f45047315efc13bba8fa7084402a353680c).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27625124
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    --- End diff --
    
    I will rename `labelCol` to `inputCol`. Feel free to suggest better class names. I'm okay with not calling it `LabelIndexer`. How about `StringToNominal`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add StringIndexer as a featur...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/4735


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85871675
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29149/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-86130157
  
    @srowen The output column now contains ML attributes. Do you have time to make another pass?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-75681145
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27868/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by petro-rudenko <gi...@git.apache.org>.
Github user petro-rudenko commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27399968
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  // TODO: handle unseen labels
    +
    +  override def fit(dataset: DataFrame, paramMap: ParamMap): LabelIndexerModel = {
    +    val map = this.paramMap ++ paramMap
    +    val counts = dataset.select(map(labelCol)).map(_.getString(0)).countByValue()
    +    val labels = counts.toSeq.sortBy(-_._2).map(_._1).toArray
    --- End diff --
    
    Maybe it makes sense to use implementation from [DatasetIndexer](https://github.com/apache/spark/pull/3000/) or vise versa, so to keep the transformation logic in one place, and if there would be need to optimize performance (e.g. get the advantages of columnar storages - some storages can provide column cardinality metadata) - to change it in one place.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-91491588
  
      [Test build #30021 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30021/consoleFull) for   PR 4735 at commit [`d82575f`](https://github.com/apache/spark/commit/d82575fd9a29149ecd5979baa70091d567c3c5bd).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class StringIndexer extends Estimator[StringIndexerModel] with StringIndexerBase `
    
     * This patch does not change any dependencies.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4735#issuecomment-85769359
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29127/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r25241949
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,115 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.param._
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{IntegerType, StringType, StructField, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    // TODO: Store ML attributes in metadata.
    +    val outputFields = inputFields :+ StructField(map(outputCol), IntegerType, nullable = false)
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an integer column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * The most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    +
    +  /** @group setParam */
    +  def setOutputCol(value: String): this.type = set(outputCol, value)
    +
    +  // TODO: Allow unseen labels in transform.
    +
    +  override def fit(dataset: DataFrame, paramMap: ParamMap): LabelIndexerModel = {
    +    val map = this.paramMap ++ paramMap
    +    val counts = dataset.select(map(labelCol)).map(_.getString(0)).countByValue()
    +    val labels = counts.toSeq.sortBy(-_._2).map(_._1).toArray
    +    val model = new LabelIndexerModel(this, map, labels)
    +    Params.inheritValues(map, this, model)
    +    model
    +  }
    +
    +  override def transformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    validateAndTransformSchema(schema, paramMap)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * Model fitted by [[LabelIndexer]].
    + */
    +@AlphaComponent
    +class LabelIndexerModel private[ml] (
    +    override val parent: LabelIndexer,
    +    override val fittingParamMap: ParamMap,
    +    labels: Array[String]) extends Model[LabelIndexerModel] with LabelIndexerBase {
    --- End diff --
    
    Open-ended question: if `labels` changes from run to run (e.g. more labels are added) then the numbering may change. The label for 0 may be different from run to run. Is that going to be a surprise later? If the indexing is only transient and is never persisted anywhere, it doesn't matter, but, might it be? I'm afraid of reloading a model referring to label 0, 1, 2 that have entirely different meaning.
    
    Maybe there are reasons this is not an issue, or the assumption is that the caller only appends values to the end of the `labels` array, ever.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-5886][ML] Add label indexer

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4735#discussion_r27628773
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/LabelIndexer.scala ---
    @@ -0,0 +1,126 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.feature
    +
    +import org.apache.spark.SparkException
    +import org.apache.spark.annotation.AlphaComponent
    +import org.apache.spark.ml.{Estimator, Model}
    +import org.apache.spark.ml.attribute.NominalAttribute
    +import org.apache.spark.ml.param._
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.sql.functions._
    +import org.apache.spark.sql.types.{StringType, StructType}
    +import org.apache.spark.util.collection.OpenHashMap
    +
    +/**
    + * Base trait for [[LabelIndexer]] and [[LabelIndexerModel]].
    + */
    +private[feature] trait LabelIndexerBase extends Params with HasLabelCol with HasOutputCol {
    +
    +  /** Validates and transforms the input schema. */
    +  protected def validateAndTransformSchema(schema: StructType, paramMap: ParamMap): StructType = {
    +    val map = this.paramMap ++ paramMap
    +    val labelType = schema(map(labelCol)).dataType
    +    require(labelType == StringType, s"The label column must be string-typed but got $labelType.")
    +    val inputFields = schema.fields
    +    val outputColName = map(outputCol)
    +    require(inputFields.forall(_.name != outputColName),
    +      s"Output column $outputColName already exists.")
    +    val attr = NominalAttribute.defaultAttr.withName(map(outputCol))
    +    val outputFields = inputFields :+ attr.toStructField()
    +    StructType(outputFields)
    +  }
    +}
    +
    +/**
    + * :: AlphaComponent ::
    + * A label indexer that maps a string column of labels to an ML column of label indices.
    + * The indices are in [0, numLabels), ordered by label frequencies.
    + * So the most frequent label gets index 0.
    + */
    +@AlphaComponent
    +class LabelIndexer extends Estimator[LabelIndexerModel] with LabelIndexerBase {
    +
    +  /** @group setParam */
    +  def setLabelCol(value: String): this.type = set(labelCol, value)
    --- End diff --
    
    Any thoughts about generalizing it beyond String?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org