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 enabl
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 does
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
ena
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 and
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(Iterable> edge,
Iterable> edg
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 ha
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 kee
Github user shghatge commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-113147138
Updated the PR by changing the removeVertices(DataSet>)
method access from Public to Private. It is only used as a helper function for
the difference method.
---
If y
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 meth
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
Differ
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 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 whic
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-112095145
Hi @andralungu, @shghatge,
I'm a bit confused by your changes.. Why did you add a new `removeVertices`
API method for DataSet?
I thought we had agreed that we'll hav
Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-112086337
Looks good to 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. If your project does not have this
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(Iterable> vertex,
Iterab
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(Iterable> vertex,
Iterab
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 @@ Graph networkWithWeights =
network.joinWithEdgesOnSource(v
+* Differenc
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/818#issuecomment-111759442
Hi @shghatge! Thank you for the PR!
I left you a couple of comments. Let us know if you have questions :)
---
If your project is set up for it, you can reply to this em
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(Iterable> edge,
Iterable> edg
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 @@ Graph networkWithWeights =
network.joinWithEdgesOnSource(v
+* Difference: Ge
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 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() method
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 proj
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 e
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
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 ena
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 c
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 from
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 te
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 te
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 te
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(Iterable> edge,
Iterable
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(Iterable> edge,
Iterable
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 @@ Graph networkWithWeights =
network.joinWithEdgesOnSource(v
* Union: Gelly's `union()`
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() method
35 matches
Mail list logo