You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2016/02/29 23:12:43 UTC
[GitHub] flink pull request: [FLINK-2788] [apis] Add TypeHint class to allo...
GitHub user StephanEwen opened a pull request:
https://github.com/apache/flink/pull/1744
[FLINK-2788] [apis] Add TypeHint class to allow type-safe generic type parsing
This adds the `TypeHint` as a type-safe way to create `TypeInformation` for generic types. I think it should gradually replace the `TypeInfoParser`.
Generic type infos can be created via
```java
TypeInformation<Tuple2<String, Double>> info = TypeInformation.of(new TypeHint<Tuple2<String, Double>>() {});
```
This creates an inline class that captures the generic parameters, and fetches them via the TypeExtractor.
*This pull request is in preparation for improving the streaming example.*
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/StephanEwen/incubator-flink typehint
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1744.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1744
----
commit c4957ce198935a390e838ca74a610108af122ad8
Author: Stephan Ewen <se...@apache.org>
Date: 2016-02-29T18:24:34Z
[FLINK-2788] [apis] Add TypeHint class to allow type-safe generic type parsing
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2788] [apis] Add TypeHint class to allo...
Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1744#issuecomment-190630492
Looks good beside a small comment. Do we also want to update the `returns` methods in `SingleInputUdfOperator` and `TwoInputUdfOperator`?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2788] [apis] Add TypeHint class to allo...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/1744#issuecomment-190660978
I think we might abuse the `of()` name a bit to much. Maybe `from()` or `for()` would be more appropriate here. So it would be `TypeInformation.for(new TypeHint...)`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2788] [apis] Add TypeHint class to allo...
Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/1744#discussion_r54539847
--- Diff: flink-core/src/main/java/org/apache/flink/api/common/typeinfo/TypeInformation.java ---
@@ -175,4 +176,39 @@ public boolean isSortKeyType() {
* @return true if obj can be equaled with this, otherwise false
*/
public abstract boolean canEqual(Object obj);
+
+ // ------------------------------------------------------------------------
+
+ /**
+ * Creates a TypeInformation for the type described by the given class.
+ *
+ * <p>This method only works for non-generic types. For generic types, use the
+ * {@link #of(TypeHint)} method.
+ *
+ * @param typeClass The class of the type.
+ * @param <T> The generic type.
+ *
+ * @return The TypeInformation object for the type described by the hint.
+ */
+ public static <T> TypeInformation<T> of(Class<T> typeClass) {
--- End diff --
Should we already add an annotation here? I think this will be public API, right?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2788] [apis] Add TypeHint class to allo...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1744#issuecomment-190662079
I am pretty much emotionless when it comes to `of`vs `from`. I think `for`does not work, it is a reserved keyword in both Java and Scala...
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2788] [apis] Add TypeHint class to allo...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1744#issuecomment-190655939
Good catch, I'll update the annotations.
I would like to add a `returns(TypeHint)` method and deprecate the String variant in a separate issue.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2788] [apis] Add TypeHint class to allo...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/1744#issuecomment-190661279
But otherwise, big +1 for this change. :smile:
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2788] [apis] Add TypeHint class to allo...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1744
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---