Skip to content

Conversation

@animenon
Copy link
Contributor

@animenon animenon commented Nov 1, 2017

What changes were proposed in this pull request?

Description added to better understand example.

How was this patch tested?

Not required as only comments are required.

Please review http://spark.apache.org/contributing.html before opening a pull request.

@animenon animenon changed the title Added description Added description to python spark Pi example Nov 1, 2017
.getOrCreate()


# If no arguments are passed(i.e. `len(sys.argv) < = 1` )
Copy link
Member

Choose a reason for hiding this comment

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

This isn't visible to end users, so don't know if this matters. It's already documented in the usage. I am not sure this meaningfully helps.

Copy link
Contributor

Choose a reason for hiding this comment

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

So I think we should expect folks to read the examples in addition to running them. That being said I don't think we need this comment specifically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually the first example on the spark doc and I wanted to know how the pi calculation was done. There was no mention of what algorithm is used for it, so took me a while to figure out the Monte-Carlo estimator was used and the logic is randomly generating over 100000 points to finally estimate the Pi value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@srowen Added a line to the index.md as well, I think it would help the curious ones know what algorithm is used and why an argument(i.e. 10) is passed.

@holdenk
Copy link
Contributor

holdenk commented Nov 2, 2017

Thanks for helping out with the Spark project, it's great to see folks looking to improve the examples :) I'm not sure the in-line comment adds much, but the docstring one looks like a good minor improvement :)

@SparkQA
Copy link

SparkQA commented Nov 2, 2017

Test build #3975 has finished for PR 19632 at commit 74e9325.

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

Description added for SparkPi example
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Sorry, this still has several problems and doesn't add much. Close it please

"""
Usage: pi [partitions]
Monte Carlo method is used to estimate Pi in the below example.
Copy link
Member

Choose a reason for hiding this comment

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

below example -> example below

.appName("PythonPi")\
.getOrCreate()

# If no arguments are passed(i.e. `len(sys.argv) < = 1` )
Copy link
Member

Choose a reason for hiding this comment

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

Missing space after passed, problem in "< ="
This comment just restates the code below.

@srowen srowen mentioned this pull request Nov 6, 2017
@asfgit asfgit closed this in ed1478c Nov 7, 2017
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.

4 participants