[Spark-7446][MLLIB] Add inverse transform for string indexer

[holdenk]

It is useful to convert the encoded indices back to their string representation for result inspection. We can add a function which creates an inverse transformation.

[SparkQA]

Test build #33308 has finished for PR 6339 at commit 06e1e35.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #33312 has finished for PR 6339 at commit 12c13f6.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[dbtsai]

This will be useful. But should be have inverse transform api in the parent class?

[holdenk]

@mengxr - should we put this in the parent class or maybe just add a trait for transformers which are invertible?

[SparkQA]

Test build #36733 has finished for PR 6339 at commit 88779c1.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #36740 has finished for PR 6339 at commit 557bef8.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[holdenk]

@mengxr : Does the inverse() method returning a separate transformer seem ok?

[holdenk]

jenkins, retest this please.

[SparkQA]

Test build #40 has finished for PR 6339 at commit 557bef8.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #37919 has finished for PR 6339 at commit 557bef8.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[jkbradley]

This will be very useful. For the design, I don't think it actually needs to be linked to StringIndexer at all since everything it needs is stored in the metadata (by StringIndexer). It should be able to take an input column of type Double (or other numerics too?), look at the metadata for a mapping, and convert the column to the corresponding string labels.

It could still be called InverseStringIndexer for clarity, though.

[jkbradley]

@holdenk Can you please add a description (e.g. copied from the JIRA description)? We're trying to enforce that for PRs. Thanks

[holdenk]

Oh sure. I will add that. I am about to get on a flight to a conference so
might take a bit of time.

On Thursday, July 23, 2015, jkbradley notifications@github.com wrote:

@holdenk https://github.com/holdenk Can you please add a description
(e.g. copied from the JIRA description)? We're trying to enforce that for
PRs. Thanks

—
Reply to this email directly or view it on GitHub
#6339 (comment).

Cell : 425-233-8271
Twitter: https://twitter.com/holdenkarau
Linked In: https://www.linkedin.com/in/holdenkarau

[jkbradley]

OK no problem

[holdenk]

Updated.

[jkbradley]

This is confusing. How about writing:

Note: By default we keep the original columns during this transformation, so the inverse should only be used on new columns such as predicted labels.

[jkbradley]

Please make the PR description match the PR. ("We can add a parameter to StringIndexer/StringIndexModel for this." is no longer quite what the PR is doing.)

[dbtsai]

Are we going to have invert or unapply in the Transformer trait? In scala, there is an extractor pattern http://docs.scala-lang.org/tutorials/tour/extractor-objects.html , and unapply will return Option since some of the transformation is one way. But Option will not be friendly for Java.

[jkbradley]

I don't think there are enough use cases for a general invert/unapply. If more real use cases come up, we can revisit this.

[jkbradley]

@holdenk The remaining issues are:

• Add explanation to doc which is built into the labels Param. (Explain that, if labels are not given, they will be taken from the inputCol metadata.)
• Switch away from using null. I confirmed we're trying to avoid the usage.
• The scala style nit above

Thanks!

[holdenk]

Ok! If instead of null would it be ok to use option and have the setter set as Some()? I feel like using an empty array as the default value is pretty ugly, but I understand if thats not a good enough reason to use an option (even if we hide it from the user).

[jkbradley]

Since the public API can't use Option, I'd prefer we just stick with an empty array. Users should not really even need to set it to an empty array since that will be the default, so hopefully it won't bother them much.

[holdenk]

ok, will do.

[SparkQA]

Test build #39166 has finished for PR 6339 at commit 64dd3a3.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #39182 has finished for PR 6339 at commit 9e241d8.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #39183 has finished for PR 6339 at commit 6a38edb.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #39253 has finished for PR 6339 at commit b9cffb6.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[holdenk]

@jkbradley should have the changes you asked for now.

[SparkQA]

Test build #39255 has finished for PR 6339 at commit 7cdf915.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[jkbradley]

LGTM once we get the tests working

Jenkins test this please

[SparkQA]

Test build #1266 has finished for PR 6339 at commit 7cdf915.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[jkbradley]

Merging with master

[billhao]

I built the master branch on my cluster yesterday and was trying to use the feature added in this pull request in PySpark. But I got an error message from the following code. Is there anything else I need to do to use this in PySpark? Or am I using it in a wrong way?

labelIndexer = StringIndexer(inputCol="labelStr", outputCol="label")
indexer = labelIndexer.fit(df)
indexer.invert(inputCol="prediction", outputCol="predictionStr")

AttributeError: 'StringIndexer' object has no attribute 'invert'

Not sure if this is the best place for the question. Let me know if there is a best place to ask this.

[jkbradley]

It's failing because invert is only in StringIndexer, not StringIndexerModel.
That's a good point though; we should probably add invert() to StringIndexerModel as well. @holdenk Would you mind sending a quick PR to do that? Thanks!

[holdenk]

@jkbradley for sure :)

[holdenk]

Wait @jkbradley its actually only implemented on StringIndexerModel which is where it belongs, but it just was not add it to the PySpark API. I could add it to the StringIndexer directly as well (provided the labels were supplied). I've created a JIRA for adding this to PySpark as well.

[jkbradley]

Ohh, right. I need to read more carefully...
I'll watch that JIRA.