Abacn commented on code in PR #40081:
URL: https://github.com/apache/beam/pull/40081#discussion_r4026992632
##########
sdks/python/apache_beam/typehints/schemas.py:
##########
@@ -374,7 +379,17 @@ def typing_to_runner_api(self, type_: type) ->
schema_pb2.FieldType:
element_type=schema_pb2.FieldType(
atomic_type=PRIMITIVE_TO_ATOMIC_TYPE[int])))
- elif _safe_issubclass(type_, Sequence) and not _safe_issubclass(type_,
str):
+ elif _safe_issubclass(type_, tuple) and not match_is_named_tuple(type_):
+ arg_types = _get_args(type_)
+ if len(arg_types) == 2 and arg_types[1] is Ellipsis: # Tuple[typ, ...]
Review Comment:
I tend to prefer not add this into `update_compatibility_version` as the
current behavior is considered semantically wrong in both scenario (on
deserialize, homogeneous tuple became list, heterogeneous tuple crash).
This is different from #37728 where the old behavior is a fallback. It also
does not affect the fallback scenario where typehint is absent and uses
FastPrimitiveCoder (#40079 case 1b, 2b)
Also update compatibility is partly mitigated by keeping encoded form of
typehint `tuple[type, ...]` the same for this change
The only scenario that could break update compatibility is the typehint
`tuple[typeA, typeA]` previously work by coincidence and backed by ArrayType,
now it's backed by RowType.
--
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]
