Skip to content

[SPARK-15064][ML] Locale support in StopWordsRemover #12968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
burakkose wants to merge 3 commits into from
Closed

[SPARK-15064][ML] Locale support in StopWordsRemover #12968

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
59c5611
locale support to StopWords
burakkose May 6, 2016
5263cbd
Merge branch 'master' into 15064
burakkose May 13, 2016
13f99d9
add return type and reference
burakkose May 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
add return type and reference
  • Loading branch information
@burakkose
burakkose committed May 13, 2016
commit 13f99d9ba175cf5cf5d1c11a8a3023b86d8e7cbd
3 changes: 2 additions & 1 deletion mllib/src/main/scala/org/apache/spark/ml/feature/StopWordsRemover.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class StopWordsRemover(override val uid: String)
/**
* Locale for doing a case sensitive comparison
* Default: English locale ("en")
Copy link
Contributor

@hhbyyh hhbyyh May 9, 2016

Choose a reason for hiding this comment

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

Shall we list what're the available options, or provide some reference here?

Copy link
Contributor Author

@burakkose burakkose May 13, 2016

Choose a reason for hiding this comment

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

It's done

* @see [[http://www.localeplanet.com/java/]]
Copy link
Contributor

@mengxr mengxr Jun 22, 2016

Choose a reason for hiding this comment

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

Please link to the official Java doc: https://docs.oracle.com/javase/8/docs/api/java/util/Locale.html or the Locale class.

* @group param
*/
val locale: Param[String] = new Param[String](this, "locale",
Copy link
Member

@srowen srowen May 7, 2016

Choose a reason for hiding this comment

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

Hm, shouldn't all this perhaps be linked to the stopwords set? if you loaded the French stopwords you'd want the French locale always?

Copy link
Contributor Author

@burakkose burakkose May 7, 2016

Choose a reason for hiding this comment

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

Yes, but, How can we know that users loaded the French stopwords? User can load stopwords by
StopWordsRemover.loadDefaultStopWords("french")
and setting is
new StopWordsRemover().setStopWords(stopWords)
. Do you have any suggestion about that case?

Copy link
Member

@srowen srowen May 8, 2016

Choose a reason for hiding this comment

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

For supported languages, we can know the appropriate locale and maintain an internal mapping. So "french" is known to map to Locale.FRENCH. For loading an arbitrary list, we don't know, but you could provide an overload where you provide a Locale.

Expand Down Expand Up @@ -126,7 +127,7 @@ class StopWordsRemover(override val uid: String)
object StopWordsRemover extends DefaultParamsReadable[StopWordsRemover] {

private[feature]
def loadLocale(value : String) = new Locale(value)
def loadLocale(value : String): java.util.Locale = new Locale(value)
Copy link
Contributor

@mengxr mengxr Jun 22, 2016

Choose a reason for hiding this comment

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

This is just new Locale(value). Shall we remove this method?

val supportedLanguages = Set("danish", "dutch", "english", "finnish", "french", "german",
"hungarian", "italian", "norwegian", "portuguese", "russian", "spanish", "swedish", "turkish")

Expand Down