Skip to content

[SPARK-12042] Python API for mllib.stat.test.StreamingTest - #11374

Closed
yinxusen wants to merge 7 commits into
apache:masterfrom
yinxusen:SPARK-12042
Closed

[SPARK-12042] Python API for mllib.stat.test.StreamingTest#11374
yinxusen wants to merge 7 commits into
apache:masterfrom
yinxusen:SPARK-12042

Conversation

@yinxusen

@yinxusenyinxusen commented Feb 25, 2016

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

The patch adds python API for mllib.stat.test.StreamingTest under JIRA https://issues.apache.org/jira/browse/SPARK-12042.

Note that for StreamingTestResult, unlike other test results in Python, I define it as a normal Python class which doesn't extend from TestResult with a _java_obj in it.

How was this patch tested?

The patch is tested with Python unit test.

@mengxr

mengxr commented Feb 25, 2016

Copy link
Copy Markdown
Contributor

cc: @feynmanliang

@SparkQA

SparkQA commented Feb 25, 2016

Copy link
Copy Markdown

Test build #51989 has finished for PR 11374 at commit 770703b.

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

@yinxusen

yinxusen commented Mar 5, 2016

Copy link
Copy Markdown
ContributorAuthor

test it please

@SparkQA

SparkQA commented Mar 6, 2016

Copy link
Copy Markdown

Test build #52523 has finished for PR 11374 at commit e4e8d5e.

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

@yinxusen

yinxusen commented Mar 6, 2016

Copy link
Copy Markdown
ContributorAuthor

cc @feynmanliang

@SparkQA

SparkQA commented May 14, 2016

Copy link
Copy Markdown

Test build #58598 has finished for PR 11374 at commit e4e8d5e.

  • This patch fails R style tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@zsxwing

zsxwing commented Oct 24, 2016

Copy link
Copy Markdown
Member

Any updates to this PR?

@SparkQA

SparkQA commented Oct 28, 2016

Copy link
Copy Markdown

Test build #67696 has finished for PR 11374 at commit 615fbbb.

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

@yinxusen

yinxusen commented Oct 28, 2016

Copy link
Copy Markdown
ContributorAuthor

Ping @mengxr@feynmanliang@yanboliang

@feynmanliang

feynmanliang commented Oct 28, 2016

Copy link
Copy Markdown
Contributor

I'll review this tonight

@feynmanliang

feynmanliang commented Oct 29, 2016

Copy link
Copy Markdown
Contributor

Apologies for the delay, I am traveling but I'll get this done this weekend.

feynmanliang
feynmanliang suggested changes Oct 29, 2016
View reviewed changes

@feynmanliangfeynmanliang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did a first pass.

It's been awhile since I've looked at PySpark so I may be a bit rusty on some things.

Comment threadexamples/src/main/python/mllib/streaming_test_example.py

"""
Create a DStream that contains several RDDs to show the StreamingTest of PySpark.
"""

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like other examples are including a from __future__ import print_function here

Comment threadexamples/src/main/python/mllib/streaming_test_example.py
sc = SparkContext(appName="PythonStreamingTestExample")
ssc = StreamingContext(sc, 1)

checkpoint_path = tempfile.mkdtemp()

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this necessary?

Comment threadexamples/src/main/python/mllib/streaming_test_example.py
from pyspark.mllib.stat.test import BinarySample, StreamingTest

if __name__ == "__main__":

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: don't include newline here

Comment threadexamples/src/main/python/mllib/streaming_test_example.py

from pyspark import SparkContext
from pyspark.streaming import StreamingContext
from pyspark.mllib.stat.test import BinarySample, StreamingTest

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$example on$ and $example off appear to be used in other examples, though I'm not sure why myself

Comment threadexamples/src/main/python/mllib/streaming_test_example.py
ssc.checkpoint(checkpoint_path)

# Create the queue through which RDDs can be pushed to a QueueInputDStream.
rdd_queue = []

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use camelCase to be consistent with other examples

Comment threadpython/pyspark/mllib/tests.py
"""

checkpoint_path = tempfile.mkdtemp()
self.ssc.checkpoint(checkpoint_path)

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this necessary?

Comment threadpython/pyspark/mllib/tests.py
input_stream = self.ssc.queueStream(rdd_queue)

model = StreamingTest()
model.setPeacePeriod(1)

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we break this into another test just for model params like

spark/python/pyspark/mllib/tests.py

Line 1165 in 39e2bad

deftest_model_params(self):
?

Comment threadmllib/src/main/scala/org/apache/spark/mllib/api/python/PythonMLLibAPI.scala
}
}

private[python] class StreamingTestResultPickler extends BasePickler[StreamingTestResult] {

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to test these in PythonMLLibAPISuite?

Comment threadpython/pyspark/mllib/stat/test.py
streamingTest.setTestMethod(self._testMethod)

javaDStream = sc._jvm.SerDe.pythonToJava(data._jdstream, True)
testResult = streamingTest.registerStream(javaDStream)

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need pythonToJava and javaToPython; its not used for streaming K means

spark/python/pyspark/mllib/clustering.py

Line 773 in 39e2bad

updatedModel=callMLlibFunc(

Comment threadmllib/src/main/scala/org/apache/spark/mllib/stat/test/TestResult.scala
*/
@Since("1.6.0")
private[stat] class StreamingTestResult @Since("1.6.0") (
class StreamingTestResult @Since("1.6.0") (

@feynmanliangfeynmanliangOct 29, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this need to be public? Java API doesn't seem to require it

@HyukjinKwon

HyukjinKwon commented Feb 9, 2017

Copy link
Copy Markdown
Member

Hi @yinxusen, are you able to proceed this further? If not, it seems it might be better closed for now.

@HyukjinKwonHyukjinKwon mentioned this pull request Feb 15, 2017
Closed
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
@HyukjinKwon@zifeif2
[BUILD] Close stale PRs
cccf33b
## What changes were proposed in this pull request?
This PR proposes to close stale PRs.
What I mean by "stale" here includes that there are some review comments by reviewers but the author looks inactive without any answer to them more than a month.
I left some comments roughly a week ago to ping and the author looks still inactive in these PR below
These below includes some PR suggested to be closed and a PR against another branch which seems obviously inappropriate.
Given the comments in the last three PRs below, they are probably worth being taken over by anyone who is interested in it.
Closesapache#7963Closesapache#8374Closesapache#11192Closesapache#11374Closesapache#11692Closesapache#12243Closesapache#12583Closesapache#12620Closesapache#12675Closesapache#12697Closesapache#12800Closesapache#13715Closesapache#14266Closesapache#15053Closesapache#15159Closesapache#15209Closesapache#15264Closesapache#15267Closesapache#15871Closesapache#15861Closesapache#16319Closesapache#16324Closesapache#16890Closesapache#12398Closesapache#12933Closesapache#14517
## How was this patch tested?
N/A
Author: hyukjinkwon <gurwls223@gmail.com>
Closesapache#16937 from HyukjinKwon/stale-prs-close.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@feynmanliangfeynmanliangfeynmanliang requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@yinxusen@mengxr@SparkQA@zsxwing@feynmanliang@HyukjinKwon