Skip to content

Conversation

@vijaykiran
Copy link
Contributor

Updates the param, in python mllib's regression.py, descriptions to be consistent. See [SPARK-11219] for
more details.

@vijaykiran vijaykiran changed the title [SPARK-12633][DOC] Update param descriptions [SPARK-12633][Python][MLlib][DOC] Update param descriptions Jan 7, 2016
@jkbradley
Copy link
Member

I just added a note to the parent JIRA about a formatting issue affecting all 5 PRs: [https://issues.apache.org/jira/browse/SPARK-11219?focusedCommentId=15090225&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15090225]
Could you please check it out & ping when I should review again? Thank you!

@vijaykiran
Copy link
Contributor Author

ping @jkbradley !

Copy link
Member

Choose a reason for hiding this comment

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

Capitalize the first word and add a period at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean "pyspark.mllib" should be "Pyspark" ? I thought package names should be left alone.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, my bad - it is already capitalized, my font was too small I guess :). Could you just add a period after the end of the description? So should read ..."scipy.sparse column matrix)."

@BryanCutler
Copy link
Member

Thanks @vijaykiran , just a couple minor corrections and would you mind changing the PR title to indicate this is for regression.py?

@vijaykiran vijaykiran changed the title [SPARK-12633][Python][MLlib][DOC] Update param descriptions [SPARK-12633][Python][MLlib][DOC] Update param descriptions in regreesion.py Jan 23, 2016
@vijaykiran vijaykiran changed the title [SPARK-12633][Python][MLlib][DOC] Update param descriptions in regreesion.py [SPARK-12633][Python][MLlib][DOC] Update param descriptions in regression.py Jan 23, 2016
Updates the param descriptions to be consistent. See [SPARK-11219] for
more details.
Change fill-column to 100.
@vijaykiran
Copy link
Contributor Author

@BryanCutler Fixed the indentation and added period.

@mengxr
Copy link
Contributor

mengxr commented Jan 27, 2016

ok to test

@SparkQA
Copy link

SparkQA commented Jan 27, 2016

Test build #50151 has finished for PR 10600 at commit 5feecba.

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

@BryanCutler
Copy link
Member

we need to settle on the :Allowed values: format, also in classification.py, before we can finish this one up.

@MLnick
Copy link
Contributor

MLnick commented Feb 23, 2016

@BryanCutler it looks like a version of the "allowed values" got merged in the PR for classification.py - https://github.com/apache/spark/pull/11183/files#diff-3a683f424bec70006f6c225a32d162f9R297?

@BryanCutler
Copy link
Member

Yeah, we were discussing the format of "allowed values" in that PR here I just wanted to make sure we used the same format for the similar params here. I'm not too crazy about the extra blank lines, but Sphinx requires them to format it correctly without generating errors.

@MLnick
Copy link
Contributor

MLnick commented Feb 26, 2016

@BryanCutler ok, from that comment discussion it seems the format agreed would be:

Allowed values:

- "l1" ...
- "l2" ...

As we discussed in the PR for tree.py, I prefer Supported values over Allowed values, though that would require updating it here and in classification.py (and #10598 has been merged already).

@BryanCutler
Copy link
Member

@MLnick I don't have a preference for Supported values vs Allowed values but I would like to see it consistent throughout, so it should probably be updated in classification.py. Should I do that here, or open another issue for that?

@MLnick
Copy link
Contributor

MLnick commented Feb 26, 2016

It's pretty minor so just add it here
On Fri, 26 Feb 2016 at 21:31, Bryan Cutler [email protected] wrote:

@MLnick https://github.com/MLnick I don't have a preference for Supported
values vs Allowed values but I would like to see it consistent
throughout, so it should probably be updated in classification.py. Should
I do that here, or open another issue for that?


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

@asfgit asfgit closed this in 236e3c8 Feb 29, 2016
roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
…sistent format

Part of task for [SPARK-11219](https://issues.apache.org/jira/browse/SPARK-11219) to make PySpark MLlib parameter description formatting consistent. This is for the regression module.  Also, updated 2 params in classification to read as `Supported values:` to be consistent.

closes apache#10600

Author: vijaykiran <[email protected]>
Author: Bryan Cutler <[email protected]>

Closes apache#11404 from BryanCutler/param-desc-consistent-regression-SPARK-12633.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants