[GitHub] flink issue #3434: [FLINK-5909] [gelly] Interface for GraphAlgorithm results

2017-03-09 Thread vasia
Github user vasia commented on the issue:

https://github.com/apache/flink/pull/3434
  
Thanks @greghogan. Looks good!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #3434: [FLINK-5909] [gelly] Interface for GraphAlgorithm results

2017-03-08 Thread greghogan
Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/3434
  
@vasia, I created `PrintableResult` with a `toPrintableString` method and 
is now used for both algorithms and analytics.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #3434: [FLINK-5909] [gelly] Interface for GraphAlgorithm results

2017-03-06 Thread vasia
Github user vasia commented on the issue:

https://github.com/apache/flink/pull/3434
  
Does `AnalyticResult` also need a method like `toVerboseString()`? Could we 
replace both with a e.g. `Result` type?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #3434: [FLINK-5909] [gelly] Interface for GraphAlgorithm results

2017-03-06 Thread greghogan
Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/3434
  
@vasia, thinking again on `AnalyticResult`, as an empty interface I could 
simply replace with `GraphAnalytic` with 
`GraphAnalytic`, for example 
[here](https://github.com/apache/flink/pull/3294/files#diff-fe7c1ff55b1b1e4cd1ee7809933607b5R60).
 Thoughts?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #3434: [FLINK-5909] [gelly] Interface for GraphAlgorithm results

2017-03-06 Thread vasia
Github user vasia commented on the issue:

https://github.com/apache/flink/pull/3434
  
Thanks for the clarification @greghogan. +1 from me.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #3434: [FLINK-5909] [gelly] Interface for GraphAlgorithm results

2017-03-05 Thread greghogan
Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/3434
  
Thanks for the review @vasia. The Unary/Binary/TertiaryResult interfaces 
will allow follow-on algorithms to work with POJOs or non-conforming Tuples 
(where we don't assume source and target vertices are fields 0 and 1). A common 
AnalyticResult interface is helpful with drivers that store both directed and 
undirected results.

The improvements to the library methods docs do reflect an expectation of 
this 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
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #3434: [FLINK-5909] [gelly] Interface for GraphAlgorithm results

2017-03-04 Thread vasia
Github user vasia commented on the issue:

https://github.com/apache/flink/pull/3434
  
Hi @greghogan, thank you for the PR.

I didn't spot anything that needs fixing, but I'm wondering what's the 
motivation to add these interfaces. I see how `toVerboseString()` is useful, 
but not really why `AnalyticResult` is needed. Also, why introduce  
`UnaryResult`, `BinaryResult`, and `TertiaryResult` instead of simply using 
tuple types?

I also see that this PR contains no changes to the docs and that the 
current 1.3-SNAPSHOT docs already reflect the changes of this PR. What am I 
missing here?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---