[GitHub] flink issue #2527: [FLINK-4624] Allow for null values in Graph Summarization
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
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
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/2527 `ToNullValue` is now committed in a new `translator` package in Gelly. I think that one will be generally very useful. I created FLINK-4673 so we can look at fixing the type extraction with the Either type. Using the recently added `TypeInfoFactory` it was quite simple and I will create the PR after tests pass (I also removed the explicit code from `TypeExtractor`). --- 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
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
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
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
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
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
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. ---