chadrik commented on a change in pull request #11632:
URL: https://github.com/apache/beam/pull/11632#discussion_r421796648



##########
File path: sdks/python/apache_beam/dataframe/transforms.py
##########
@@ -248,7 +261,12 @@ def _dict_union(dicts):
   return result
 
 
-def _flatten(valueish, root=()):
+def _flatten(
+    valueish,  # type: Union[T, Tuple[T, ...], Dict[Any, T]]

Review comment:
       do we want to cover the `List[T]` case for `valueish`?  
   
   Alternately, do we wan to make this more generic:
   
   ```python
   def _flatten(
       valueish,  # type: Union[T, Iterable[T], Mapping[Any, T]]
       root=(),  # type: Tuple[Any, ...]
       ):
     # type: (...) -> Dict[Tuple[Any, ...], T]
     """Given a nested structure of dicts, tuples, and lists, return a flat
     dictionary where the values are the leafs and the keys are the "paths" to
     these leaves.
   
     For example `{a: x, b: (y, z)}` becomes `{(a,): x, (b, 0): y, (b, 1): c}`.
     """
     if isinstance(valueish, typing.Mapping):
       return _dict_union(_flatten(v, root + (k, )) for k, v in 
valueish.items())
     elif isinstance(valueish, typing.Iterable):
       return _dict_union(
           _flatten(v, root + (ix, )) for ix, v in enumerate(valueish))
     else:
       return {root: valueish}
   ```
   
   Another thought, is it valid / worthwhile to create a relationship between 
`valueish`, `root`, and the result key?:
   
   ```python
   def _flatten(
       valueish,  # type: Union[T, Iterable[T], Mapping[U, T]]
       root=(),  # type: Tuple[U, ...]
       ):
     # type: (...) -> Dict[Tuple[U, ...], T]
   ```
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to