Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1349#discussion_r45545495
--- Diff:
flink-staging/flink-table/src/test/java/org/apache/flink/api/java/table/test/CastingITCase.java
---
@@ -44,29 +41,13 @@ public CastingITCase
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1349#discussion_r45545498
--- Diff:
flink-staging/flink-table/src/test/java/org/apache/flink/api/java/table/test/CastingITCase.java
---
@@ -89,7 +68,7 @@ public void
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1349#discussion_r45545501
--- Diff:
flink-staging/flink-table/src/test/java/org/apache/flink/api/java/table/test/ExpressionsITCase.java
---
@@ -98,11 +77,9 @@ public void
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1349#discussion_r45545505
--- Diff:
flink-staging/flink-table/src/test/java/org/apache/flink/api/java/table/test/FilterITCase.java
---
@@ -73,11 +54,9 @@ public void
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1337#issuecomment-158631602
@StephanEwen @mxm 7h4nk5 f0r r3v13w. If there is no objection, I would like
to merge this.
---
If your project is set up for it, you can reply to this email
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1349#discussion_r45545486
--- Diff:
flink-staging/flink-table/src/test/java/org/apache/flink/api/java/table/test/AsITCase.java
---
@@ -68,16 +49,14 @@ public void testAs() throws
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1349#issuecomment-157239452
Okay I'll review soon. Thanks for update.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1349#issuecomment-156902819
@gallenvara Yes, exactly right.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1349#issuecomment-156469184
Hi @gallenvara, I read your PR and have some comments.
First, since Flink 0.10, we dropped Java 6 support and can use omit types
if the types can be inferred
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/commit/dd66e61ecc5da5b15a610f04b98c8386d141f910#commitcomment-14352242
@gallenvara AFAIK JMH is licensed under GPL
(http://hg.openjdk.java.net/code-tools/jmh/file/bcec9a03787f/LICENSE). GPL
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1350#issuecomment-156142663
Hi @HilmiYildirim, Thanks for opening pull request. I would like to
shepherd this PR. Could you rename this PR to include JIRA issue number? I
think "[FLINK
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1350#issuecomment-156156202
@uce Thanks for correction. :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1349#issuecomment-156028176
I would like to shepherd this PR. Thanks for opening pull request
@gallenvara. I'll review soon.
---
If your project is set up for it, you can reply to this email
GitHub user chiwanpark opened a pull request:
https://github.com/apache/flink/pull/1337
[FLINK-2947] [scala shell] Add color support to Scala Shell
This PR adds color support to Scala Shell with Scala 2.11. Color scheme is
inherited from Scala 2.11 default color scheme
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1220#issuecomment-153096308
I would suggest logic like following:
```scala
val useQuadTree = ~~~
if (useQuadTree) {
knnQueryWithQuadTree(training, testing, out
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1220#discussion_r43647151
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/nn/KNN.scala ---
@@ -0,0 +1,297 @@
+/*
+ * Licensed to the Apache Software
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1220#issuecomment-153078409
Hi @danielblazevski, Thanks for updating your pull request. I'll review
this soon. From short review, I think that we have to split predict method into
two methods
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1220#discussion_r43646670
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/nn/KNN.scala ---
@@ -0,0 +1,297 @@
+/*
+ * Licensed to the Apache Software
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1220#issuecomment-153081706
Currently, although `useQuadTree` is set to false, the current logic
creates unnecessary bounding boxes and quad tree object. This causes
performance decreasing. So
GitHub user chiwanpark opened a pull request:
https://github.com/apache/flink/pull/1312
[FLINK-2950] [ml] [docs] Fix markdown rendering problem in SVM documentation
Because the line 174 of `svm.md` has a wrong end tag (`<\strong>`), the
documentation doesn't render pr
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1299#issuecomment-152690793
LGTM except one minor cosmetic issue. :+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1299#discussion_r43567824
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/hadoop/mapred/HadoopInputFormat.scala
---
@@ -27,6 +27,10 @@ class HadoopInputFormat[K, V
GitHub user chiwanpark opened a pull request:
https://github.com/apache/flink/pull/1310
[FLINK-2860] The mlr object from the FlinkML Getting Started code example
uses an undefined argument
This PR fixes the code in getting started guide. We have to apply this
commit to 0.9 branch
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1254#issuecomment-147244762
@ssaumitra Yes, the link is correct. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1254#issuecomment-147243883
I would like to shepherd this PR.
Hi @ssaumitra, thanks for opening pull request! But, your PR has a problem
to merge. Because the document is about FlinkML
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1106#issuecomment-146891099
Merging...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1224#issuecomment-146885004
I'll merge this with addressing @StephanEwen's comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
GitHub user chiwanpark opened a pull request:
https://github.com/apache/flink/pull/1247
[FLINK-2774] [scala shell] Import some Java and Core API classes
automatically
This PR adds some packages and classes into scala shell import list for
improving user experience. Please check
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1234#issuecomment-146891720
Merging this...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1234#issuecomment-146465380
If we need to parallelize tests run by scalatest, the test cases should
extend
[ParallelTestExecution](http://doc.scalatest.org/2.2.4/index.html
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503795
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -68,38 +94,62 @@ object FlinkShell
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503815
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -68,38 +94,62 @@ object FlinkShell
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1186#discussion_r41496701
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala
---
@@ -18,23 +18,20 @@
package
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1186#discussion_r41497880
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/regression/MultipleLinearRegression.scala
---
@@ -124,6 +121,52 @@ class
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1186#discussion_r41497844
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/regression/MultipleLinearRegression.scala
---
@@ -18,15 +18,12 @@
package
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503634
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -18,48 +18,74 @@
package
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41504281
--- Diff:
flink-staging/flink-scala-shell/src/test/scala/org/apache/flink/api/scala/ScalaShellITSuite.scala
---
@@ -225,8 +223,50 @@ class
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1106#issuecomment-146519237
Hi @nikste, thanks for update. It looks good to merge except some style
issues.
---
If your project is set up for it, you can reply to this email and have your
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1186#discussion_r41498327
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/regression/MultipleLinearRegression.scala
---
@@ -124,6 +121,52 @@ class
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503746
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -68,38 +94,62 @@ object FlinkShell
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503700
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -68,38 +94,62 @@ object FlinkShell
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503724
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -68,38 +94,62 @@ object FlinkShell
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41504146
--- Diff: docs/apis/scala_shell.md ---
@@ -30,15 +30,16 @@ Flink and setting up a cluster please refer to
To use the shell with an integrated Flink
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1186#discussion_r41497773
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/regression/MultipleLinearRegression.scala
---
@@ -124,6 +121,52 @@ class
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503504
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -18,48 +18,74 @@
package
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503540
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -18,48 +18,74 @@
package
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503568
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -18,48 +18,74 @@
package
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41503496
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -18,48 +18,74 @@
package
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41504295
--- Diff:
flink-staging/flink-scala-shell/src/test/scala/org/apache/flink/api/scala/ScalaShellLocalStartupITCase.scala
---
@@ -0,0 +1,61
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41504243
--- Diff: docs/apis/scala_shell.md ---
@@ -30,15 +30,16 @@ Flink and setting up a cluster please refer to
To use the shell with an integrated Flink
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1106#issuecomment-146560991
@nikste Sorry for bothering you, but there are still some style issues.
Looks good to merge except the issues.
---
If your project is set up for it, you can reply
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41518056
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -67,39 +93,62 @@ object FlinkShell
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41518604
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkILoop.scala
---
@@ -34,16 +34,16 @@ class FlinkILoop
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41519871
--- Diff:
flink-staging/flink-scala-shell/src/test/scala/org/apache/flink/api/scala/ScalaShellITSuite.scala
---
@@ -225,8 +223,50 @@ class
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41519895
--- Diff:
flink-staging/flink-scala-shell/src/test/scala/org/apache/flink/api/scala/ScalaShellLocalStartupITCase.scala
---
@@ -0,0 +1,60
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1106#issuecomment-146533038
Okay, if there is no objection and the travis passes, I'll merge this.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41518461
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -18,48 +18,74 @@
package
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41518435
--- Diff:
flink-staging/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala
---
@@ -18,48 +18,74 @@
package
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1234#issuecomment-146578397
@StephanEwen I think so.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1186#issuecomment-146570848
Okay, We need some discussion in mailing list about ML model import/export
feature. I think that PMML support is one of sub-issues related to the ML model
import
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1220#discussion_r41526734
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/nn/QuadTree.scala ---
@@ -0,0 +1,305 @@
+
+/*
+ * Licensed to the Apache
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41523437
--- Diff:
flink-staging/flink-scala-shell/src/test/scala/org/apache/flink/api/scala/ScalaShellLocalStartupITCase.scala
---
@@ -0,0 +1,60
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1106#discussion_r41525735
--- Diff:
flink-staging/flink-scala-shell/src/test/scala/org/apache/flink/api/scala/ScalaShellITSuite.scala
---
@@ -225,8 +223,50 @@ class
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1220#discussion_r41378136
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/nn/KNN.scala ---
@@ -0,0 +1,298 @@
+/*
+ * Licensed to the Apache Software
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1220#discussion_r41378859
--- Diff:
flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/nn/QuadTree.scala ---
@@ -0,0 +1,305 @@
+
+/*
+ * Licensed to the Apache
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1220#issuecomment-146202529
It sounds weird for me. If the user sets `useQuadTree` to false, the
algorithm should not use quadtree. Otherwise if the user sets `useQuadTree` to
true
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1212#issuecomment-146213445
+1 for merging. If there is no objection, I'll merge this in few hours.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1217#discussion_r41403652
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/typeutils/UnitSerializer.scala
---
@@ -0,0 +1,58 @@
+/*
+ * Licensed
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1217#issuecomment-146233014
Looks good to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1234#issuecomment-146275842
Hi @dawidwys, Thanks for opening pull request.
Your pull request looks good to merge. This PR will be merged in few days
if there is no objection.
---
If your
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1106#issuecomment-146267428
Hi @nikste, because #1197 is merged first into master, we need rebase this
on master. Could you rebase this?
---
If your project is set up for it, you can reply
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1205#issuecomment-145970361
I'm worrying about the name of method `fromCsvReader`. It seems that the
method should receive `CsvReader` object and read graph data from the reader
object. Why we
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/696#issuecomment-145811977
Because there is another pull request #1220 for this issue, I close this.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user chiwanpark closed the pull request at:
https://github.com/apache/flink/pull/696
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1205#issuecomment-146030951
Okay, I understand it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1197#issuecomment-145606437
It occured by jline library version mismatching between Scala 2.10 and
2.11. Since Scala 2.11.5, we don't need jline dependency. Scala REPL uses its
own jline
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1221#issuecomment-145505679
I also meant the alignment. You need more newlines for `succeeded` and
`failed`.
---
If your project is set up for it, you can reply to this email and have your
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1197#issuecomment-145509803
Oh, I'll investigate it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1220#issuecomment-145364842
Okay, I'll review further in few days.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1220#issuecomment-145353515
I just reviewed quickly. I have some comments for this pull request.
1. This pull request cannot pass checking Scala code style. The Scala code
should
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1218#discussion_r41101714
--- Diff:
flink-staging/flink-tez/src/main/java/org/apache/flink/tez/runtime/TezTask.java
---
@@ -402,7 +402,7 @@ private void initInputLocalStrategy(int
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1218#issuecomment-145383808
Looks good to merge except minor issues for me. :+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1218#discussion_r41101621
--- Diff: flink-tests/src/test/resources/logback-test.xml ---
@@ -27,7 +27,7 @@
-
+
--- End diff
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1221#issuecomment-145401720
I'm not sure this patch is ready to merge. Because of additional components
for log such as time, thread name, caller class name, etc., the log output is
not same
Github user chiwanpark commented on a diff in the pull request:
https://github.com/apache/flink/pull/1212#discussion_r41009352
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/codegen/TypeInformationGen.scala
---
@@ -23,6 +23,7 @@ import
GitHub user chiwanpark opened a pull request:
https://github.com/apache/flink/pull/1197
[FLINK-2767] [scala shell] Add Scala 2.11 support to Scala shell
This PR contains following changes:
* Add `ILoopCompat` class for supporting both Scala 2.10 and Scala 2.11
(Inspired
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1197#issuecomment-144335229
AFAIK, the directory structure is planned to correct in #1106. But I didn't
know #1106 is closed. I'll update the directory structure.
---
If your project is set up
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1197#issuecomment-144336975
The directory name is changed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user chiwanpark commented on the pull request:
https://github.com/apache/flink/pull/1106#issuecomment-144341545
But It seems that there is still same problem.
