[
https://issues.apache.org/jira/browse/BEAM-14430?focusedWorklogId=769770&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769770
]
ASF GitHub Bot logged work on BEAM-14430:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 12/May/22 17:26
Start Date: 12/May/22 17:26
Worklog Time Spent: 10m
Work Description: chamikaramj commented on code in PR #17608:
URL: https://github.com/apache/beam/pull/17608#discussion_r871611650
##########
model/pipeline/src/main/proto/org/apache/beam/model/pipeline/v1/beam_runner_api.proto:
##########
@@ -1094,6 +1094,13 @@ message StandardCoders {
}
}
+message LogicalTypes {
Review Comment:
Let's add a top level comment as well.
##########
model/pipeline/src/main/proto/org/apache/beam/model/pipeline/v1/beam_runner_api.proto:
##########
@@ -1094,6 +1094,13 @@ message StandardCoders {
}
}
+message LogicalTypes {
Review Comment:
Seems like we defined other logical types in comments:
https://github.com/apache/beam/blob/fd61a90057011270dbf9a36c73b5baaf120100e2/model/pipeline/src/main/proto/org/apache/beam/model/pipeline/v1/beam_runner_api.proto#L1049
For consistency, should we move all definitions here ?
cc: @TheNeuralBit
##########
sdks/java/extensions/python/src/main/java/org/apache/beam/sdk/extensions/python/PythonExternalTransform.java:
##########
@@ -179,16 +204,20 @@ Row buildOrGetKwargsRow() {
// Types that are not one of following are considered custom types.
// * Java primitives
// * Type String
+ // * Type PythonCallableSource
+ // * Any Type explicitly annotated by withTypeHint()
// * Type Row
- private static boolean isCustomType(java.lang.Class<?> type) {
+ private boolean isCustomType(java.lang.Class<?> type) {
boolean val =
!(ClassUtils.isPrimitiveOrWrapper(type)
|| type == String.class
+ || type == PythonCallableSource.class
Review Comment:
Can this be generalized to all logical types somehow (instead of special
casing PythonCallableSource) ?
##########
sdks/java/extensions/python/src/main/java/org/apache/beam/sdk/extensions/python/PythonExternalTransform.java:
##########
@@ -162,6 +167,26 @@ public PythonExternalTransform<InputT, OutputT>
withKwargs(Row kwargs) {
return this;
}
+ /**
+ * Specifies the field type of arguments.
+ *
+ * <p>Type hints are especially useful for logical types since type
inference does not work well
+ * for logical types.
+ *
+ * @param argType A class object for the argument type.
+ * @param fieldType A schema field type for the argument.
+ * @return updated wrapper for the cross-language transform.
+ */
+ public PythonExternalTransform<InputT, OutputT> withTypeHint(
Review Comment:
Should this be per arg instead of per type ? In other words, can the same
class map to different schema types ?
Issue Time Tracking
-------------------
Worklog Id: (was: 769770)
Time Spent: 2h (was: 1h 50m)
> Adding a logical type support for Python callables to Row schema
> ----------------------------------------------------------------
>
> Key: BEAM-14430
> URL: https://issues.apache.org/jira/browse/BEAM-14430
> Project: Beam
> Issue Type: New Feature
> Components: cross-language
> Reporter: Heejong Lee
> Assignee: Heejong Lee
> Priority: P2
> Time Spent: 2h
> Remaining Estimate: 0h
>
> Adding a logical type support for Python callables to Row schema
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
