Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-115215460
Thank you @shghatge! I'll merge 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
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/818
---
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
Github user shghatge commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-114983698
Updated PR
---
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 vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/818#discussion_r33026720
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/Graph.java ---
@@ -1234,6 +1248,17 @@ public void coGroup(IterableEdgeK, EV edge,
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-114430160
Hi @shghatge!
Thank you for the quick update. Apart from my minor comment, this looks
good now :)
---
If your project is set up for it, you can reply to this email
Github user shghatge commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-113147138
Updated the PR by changing the removeVertices(DataSetVertexK, VV)
method access from Public to Private. It is only used as a helper function for
the difference method.
Github user shghatge commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-113150859
Updated the docs accordingly.
---
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
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-113149078
Hi @shghatge ,
Don't forget to remove the definition for the public
removeVertices(DataSet) from the documentation.
Up for discussion: should we
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-112923172
Hi @shghatge!
For this issue, I think we should only add the `difference` method. If you
want to avoid duplicating code, you can always define a private helper
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-112814353
Hey,
difference in not a fancy way of removing vertices, no :)
If you recall our previous conversation in #678, we decided to have
add/remove methods for small
Github user shghatge commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-112827357
Hello @vasia
Could you please elaborate on what should be the future course of action?
Should I put the content of removeVertices(DataSet) method in the
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/818#discussion_r32415140
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/Graph.java ---
@@ -1151,6 +1151,23 @@ public void coGroup(IterableVertexK, VV
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/818#discussion_r32415176
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/Graph.java ---
@@ -1151,6 +1151,23 @@ public void coGroup(IterableVertexK, VV
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/818#discussion_r32414981
--- Diff: docs/libs/gelly_guide.md ---
@@ -240,6 +240,7 @@ GraphLong, Double, Double networkWithWeights =
network.joinWithEdgesOnSource(v
img
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-112099290
Hi @vasia ,
In essence the `difference` method is just a fancy way of removing
vertices, right?
When you remove a vertex, you also remove the edge for
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/818#discussion_r32374715
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/Graph.java ---
@@ -1234,6 +1234,18 @@ public void coGroup(IterableEdgeK, EV edge,
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/818#discussion_r32374703
--- Diff: docs/libs/gelly_guide.md ---
@@ -240,6 +240,7 @@ GraphLong, Double, Double networkWithWeights =
network.joinWithEdgesOnSource(v
img alt=Union
GitHub user shghatge opened a pull request:
https://github.com/apache/flink/pull/818
[FLINK-2093][gelly] Added difference Method
Tasks given on 5th June:
Add a difference function to the Graph.java
Modify the docs 'gelly-guide.md'
Add the test case for difference()
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-110739263
Hi @shghatge,
Apart from the minor cosmetic suggestions I made, everything looks well.
@vasia, could you double check this? After Shivani simplifies
Github user shghatge commented on the pull request:
https://github.com/apache/flink/pull/807#issuecomment-110380728
Hi @fhueske I am sorry for closing the pull request. Now onwards I will
follow the approach you suggested.
---
If your project is set up for it, you can reply to this
Github user fhueske commented on the pull request:
https://github.com/apache/flink/pull/807#issuecomment-110438710
@shghatge no worries. It's not a big deal :-)
---
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 fhueske commented on the pull request:
https://github.com/apache/flink/pull/807#issuecomment-110143059
Hi @shghatge, you don't need to close a PR in order to update it.
You can simply update (push or push --force into) the branch from which you
created the PR and
Github user shghatge commented on the pull request:
https://github.com/apache/flink/pull/807#issuecomment-110078878
Then it was just removing vertices! Talk about swatting a Fly with a
Sledgehammer! I will do all the changes you suggested.
---
If your project is set up for it, you
Github user shghatge closed the pull request at:
https://github.com/apache/flink/pull/807
---
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
GitHub user shghatge opened a pull request:
https://github.com/apache/flink/pull/807
[FLINK-2093][gelly] Added difference method
Tasks given on 5th June:
Add a difference function to the Graph.java
Modify the docs 'gelly-guide.md'
Add the test case for difference()
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/807#issuecomment-110070080
Hi @shghatge ,
This very nice for a first PR and I am happy to see that you followed my
guidelines :)
I left a set of comments in-line.
Apart
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/807#discussion_r31930812
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/Graph.java ---
@@ -1233,6 +1233,34 @@ public void coGroup(IterableEdgeK, EV edge,
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/807#discussion_r31932002
--- Diff:
flink-staging/flink-gelly/src/test/java/org/apache/flink/graph/test/operations/GraphOperationsITCase.java
---
@@ -266,6 +266,47 @@ public void
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/807#discussion_r31932048
--- Diff:
flink-staging/flink-gelly/src/test/java/org/apache/flink/graph/test/operations/GraphOperationsITCase.java
---
@@ -266,6 +266,47 @@ public void
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/807#discussion_r31930244
--- Diff: docs/libs/gelly_guide.md ---
@@ -236,6 +236,8 @@ GraphLong, Double, Double networkWithWeights =
network.joinWithEdgesOnSource(v
*
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/807#discussion_r31930744
--- Diff:
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/Graph.java ---
@@ -1233,6 +1233,34 @@ public void coGroup(IterableEdgeK, EV edge,
Github user andralungu commented on a diff in the pull request:
https://github.com/apache/flink/pull/807#discussion_r31932709
--- Diff:
flink-staging/flink-gelly/src/test/java/org/apache/flink/graph/test/operations/GraphOperationsITCase.java
---
@@ -266,6 +266,47 @@ public void
32 matches
Mail list logo
