bsannicolas commented on code in PR #29849:
URL: https://github.com/apache/beam/pull/29849#discussion_r1439662692
##########
sdks/python/apache_beam/typehints/schemas.py:
##########
@@ -343,14 +343,8 @@ def typing_to_runner_api(self, type_: type) ->
schema_pb2.FieldType:
argument = None
if logical_type.argument_type() is not None:
argument_type = self.typing_to_runner_api(logical_type.argument_type())
- try:
- argument = self.value_to_runner_api(
- argument_type, logical_type.argument())
- except ValueError:
Review Comment:
Currently I only implement map type non-atomic types, but I think a full
implementation would remove expected value errors.
```def value_to_runner_api(self, typing_proto: schema_pb2.FieldType, value):
if typing_proto.WhichOneof("type_info") == "map_type":
.... // My changes
if typing_proto.WhichOneof("type_info") != "atomic_type":
# TODO: Allow other value types
raise ValueError(
"Only atomic_type option values are currently supported in Python.
"
f"Got {value!r}, which maps to fieldtype {typing_proto!r}.")
```
If we agree on this approach, I'll implement the others. Though for the the
full issue, there are some TODOs around existing logical types that I'm not
sure how to resolve (i.e. what the intended resolution would be).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
