You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by vasia <gi...@git.apache.org> on 2015/09/25 13:14:26 UTC
[GitHub] flink pull request: [FLINK-2561] Adda missing Gelly scala methods ...
GitHub user vasia opened a pull request:
https://github.com/apache/flink/pull/1183
[FLINK-2561] Adda missing Gelly scala methods and tests
This PR *partially* address FLINK-2561.
- It adds all missing methods to Graph, except the recently added (#1149) `fromCsvReader`. It is not straight-forward to port this one (e.g. with a simple wrapper), so I suggest we address it with a separate JIRA.
- It ports all existing tests to use `collect` instead of files and adds tests for the new methods.
- It adds a completeness test (where `fromCsvReader` is temporarily excluded)
- It adds the Scala version of the GraphMetrics example. For more examples to be added, #1033 and #1152 should be merged.
I will submit a separate PR for the documentation.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vasia/flink gelly-scala
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1183.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1183
----
commit c446c0ca648c9a22e241931a3ea08104e41339a8
Author: vasia <va...@apache.org>
Date: 2015-09-24T20:08:10Z
[FLINK-2561] [gelly] add missing methods to Graph:
add-remove edges/vertices, difference, graph creation methods,
validate, getTriplets.
commit 3251d19c2d77192f2f0260c00eb88cf449e26ffa
Author: vasia <va...@apache.org>
Date: 2015-09-24T20:10:25Z
[FLINK-2561] [gelly] add missing utility mappers
commit 1fb32d5dcf91611133266bcf48ae083da0c7ae44
Author: vasia <va...@apache.org>
Date: 2015-09-24T21:31:38Z
[FLINK-2561] [gelly] convert existing tests to use collect instead of files;
add tests for newly added operations
commit 0ce15f4795d174f5d7306453f94632c045ee622e
Author: vasia <va...@apache.org>
Date: 2015-09-24T22:39:04Z
[FLINK-2561] [gelly] add completeness test: fromCsvReader method is missing;
fix checkstyle violations
commit 510d9cd38f1eead6b346dc595b169cffc8e3ac67
Author: vasia <va...@apache.org>
Date: 2015-09-25T09:20:15Z
[FLINK-2561] [gelly] add GraphMetrics Scala example
----
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2561] Adds missing Gelly scala methods ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1183
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2561] Adds missing Gelly scala methods ...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1183#discussion_r40540447
--- Diff: flink-staging/flink-gelly-scala/src/main/scala/org/apache/flink/graph/scala/utils/Tuple2ToVertexMap.scala ---
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.graph.scala.utils
+
+import org.apache.flink.api.common.functions.MapFunction
+import org.apache.flink.graph.Vertex
+
+class Tuple2ToVertexMap[K, VV] extends MapFunction[(K, VV), Vertex[K, VV]] {
+
+ private val serialVersionUID: Long = 1L
--- End diff --
I think in Scala, serialVersionUID has to be specified via an annotation, like this: `@SerialVersionUID(1L)`
The serialVersionUID needs to be static, which is a concept not used by Scala. I think the way you wrote it will make the UID a simple instance member field.
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2561] Adds missing Gelly scala methods ...
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/1183#issuecomment-143725226
Thank you for the review @StephanEwen! I'll address your comments and merge later.
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2561] Adds missing Gelly scala methods ...
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/1183#discussion_r40544282
--- Diff: flink-staging/flink-gelly-scala/src/main/scala/org/apache/flink/graph/scala/utils/Tuple2ToVertexMap.scala ---
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.graph.scala.utils
+
+import org.apache.flink.api.common.functions.MapFunction
+import org.apache.flink.graph.Vertex
+
+class Tuple2ToVertexMap[K, VV] extends MapFunction[(K, VV), Vertex[K, VV]] {
+
+ private val serialVersionUID: Long = 1L
--- End diff --
Thanks! I copied this from `VertexToTuple2Map` and didn't really think about it.
I'll check whether there are more instances of this in gelly-scala and correct them.
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2561] Adds missing Gelly scala methods ...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1183#issuecomment-143714240
Looks good to me!
One style comment: I think that `collect()` should be written with parenthesis. In Scala, you use parenthesis if the methods are not purely accessing properties. The `collect()` function as effects.
Minor comment about the `serialVersionUID` in Scala inline.
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---