github-actions[bot] commented on code in PR #41049:
URL: https://github.com/apache/beam/pull/41049#discussion_r4496400664
##########
sdks/python/apache_beam/yaml/yaml_combine.py:
##########
@@ -103,11 +105,11 @@ class PyJsYamlCombine(beam.PTransform):
"""
def __init__(
self,
- group_by: Iterable[str],
- combine: Mapping[str, Mapping[str, Any]],
+ group_by: Iterable[str] = (),
Review Comment:
**🟡 Medium**: `InlineProvider.config_schema_from_callable` derives config
field nullability (used for `required` in `beam_schema_to_json_schema` and
`(Optional)` in `generate_yaml_docs.py`) directly from `Optional[...]` in the
type annotations via `typing_to_runner_api`, rather than from default values.
Annotating `group_by` as `Iterable[str]` leaves `group_by` marked as
required (`nullable=False`) in the generated schema/docs and conflicts with
passing `group_by=None`. Annotating `group_by` as `Optional[Iterable[str]] =
()` (matching `_PyJsMapToFields` in `yaml_mapping.py`) and keeping `combine`
non-`Optional` (e.g., `combine: Mapping[str, Mapping[str, Any]] = {} # pylint:
disable=dangerous-default-value`) ensures `group_by` is marked optional while
`combine` remains required in the config schema.
```suggestion
group_by: Optional[Iterable[str]] = (),
```
##########
sdks/python/apache_beam/yaml/yaml_combine.py:
##########
@@ -180,7 +182,12 @@ def extract_return_type(expr):
@beam.ptransform.ptransform_fn
def _SqlCombineTransform(
- pcoll, sql_transform_constructor, group_by, combine, language=None):
+ pcoll,
+ sql_transform_constructor,
+ group_by: Iterable[str] = (),
Review Comment:
**🟡 Medium**: Similarly here for `SqlBackedProvider.config_schema` (and for
type-checking the `group_by=None` call in `test_sql_combine_query_generation`),
`group_by` should be annotated as `Optional[Iterable[str]] = ()` and `combine`
kept non-`Optional` (`combine: Mapping[str, Any] = {} # pylint:
disable=dangerous-default-value`).
```suggestion
group_by: Optional[Iterable[str]] = (),
```
--
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]
