[GitHub] flink issue #2527: [FLINK-4624] Allow for null values in Graph Summarization

[greghogan]

Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/2527
  
Will merge today. I'll add a note that the `TypeInformation` will be 
removed once we resolve the issues typing `Either`.


---
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 #2527: [FLINK-4624] Allow for null values in Graph Summarization

[s1ck]

Github user s1ck commented on the issue:

https://github.com/apache/flink/pull/2527
  
@greghogan since I addressed all your comments, is there anything left to 
do that prevents this from 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
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 #2527: [FLINK-4624] Allow for null values in Graph Summarization

[s1ck]

Github user s1ck commented on the issue:

https://github.com/apache/flink/pull/2527
  
> I'd also like to add a ToNullValue translator that would accept any type 
and convert to NullValue.

I don't know if I get this right. Do you mean an additional test case with 
`NullValue` as vertex/edge?


---
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 #2527: [FLINK-4624] Allow for null values in Graph Summarization

[s1ck]

Github user s1ck commented on the issue:

https://github.com/apache/flink/pull/2527
  
> Would a graph translator simplify the conversion from Long to String? You 
can do graph.run(new TranslateEdgeValues<...>(new StringToLong()) and write a 
simple public class StringToLong implements TranslateFunction { 
 Or this could be an anonymous class.

I was not aware of this function / interface. I just needed it to use the 
same data for String and Long tests. I will update the conversion to your 
suggested method.


---
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 #2527: [FLINK-4624] Allow for null values in Graph Summarization

[s1ck]

Github user s1ck commented on the issue:

https://github.com/apache/flink/pull/2527
  
> I'm not following why specifying the TypeInformation is now required with 
the change to using Either. Is the type system failing to handle this properly?
You are right, I was experimenting with return type definitions for the 
groupReduce and the map call. However, for the map call in 
`Summarization.java:126` the return type definition is required or the job 
fails as it cannot determine the type for `VV`.


---
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 #2527: [FLINK-4624] Allow for null values in Graph Summarization

[greghogan]

Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/2527
  
I'm not following why specifying the `TypeInformation` is now required with 
the change to using `Either`. Is the type system failing to handle this 
properly?


---
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 #2527: [FLINK-4624] Allow for null values in Graph Summarization

[greghogan]

Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/2527
  
I'd also like to add a `ToNullValue` translator that would accept any type 
and convert to `NullValue`.


---
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 #2527: [FLINK-4624] Allow for null values in Graph Summarization

[greghogan]

Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/2527
  
Would a graph translator simplify the conversion from Long to String? You 
can do `graph.run(new TranslateEdgeValues<...>(new StringToLong())` and write a 
simple `public class StringToLong implements TranslateFunction { 
...`. Or this could be an anonymous class.


---
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.
---