[
https://issues.apache.org/jira/browse/BEAM-7746?focusedWorklogId=495497&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-495497
]
ASF GitHub Bot logged work on BEAM-7746:
----------------------------------------
Author: ASF GitHub Bot
Created on: 05/Oct/20 18:37
Start Date: 05/Oct/20 18:37
Worklog Time Spent: 10m
Work Description: robertwb commented on a change in pull request #13004:
URL: https://github.com/apache/beam/pull/13004#discussion_r499788649
##########
File path: sdks/python/apache_beam/transforms/core.py
##########
@@ -2465,22 +2470,28 @@ def expand(self, pcoll):
return pcoll | Map(lambda x: (self._key_func()(x), x)) | GroupByKey()
-_dynamic_named_tuple_cache = {}
+_dynamic_named_tuple_cache = {
+} # type: typing.Dict[typing.Tuple[str, typing.Tuple[str, ...]],
typing.Type[tuple]]
def _dynamic_named_tuple(type_name, field_names):
+ # type: (str, typing.Tuple[str, ...]) -> typing.Type[tuple]
cache_key = (type_name, field_names)
result = _dynamic_named_tuple_cache.get(cache_key)
if result is None:
import collections
result = _dynamic_named_tuple_cache[cache_key] = collections.namedtuple(
type_name, field_names)
- result.__reduce__ = lambda self: (
- _unpickle_dynamic_named_tuple, (type_name, field_names, tuple(self)))
+ if not typing.TYPE_CHECKING:
Review comment:
Is it possible to suppress the warning rather than take this branch at
runtime-only (which feels odd)?
##########
File path: sdks/python/apache_beam/transforms/core.py
##########
@@ -2394,8 +2399,8 @@ class GroupBy(PTransform):
"""
def __init__(
self,
- *fields, # type: typing.Union[str, callable]
- **kwargs # type: typing.Union[str, callable]
+ *fields, # type: typing.Union[str, typing.Callable]
Review comment:
Is there any advantage to (untyped) `typing.Callable`?
##########
File path: sdks/python/apache_beam/transforms/userstate.py
##########
@@ -305,23 +323,37 @@ def validate_stateful_dofn(dofn):
(timer_spec, method_name, dofn))
-class RuntimeTimer(object):
+class BaseTimer(object):
Review comment:
This is good.
##########
File path: sdks/python/apache_beam/transforms/stats.py
##########
@@ -426,14 +432,15 @@ def __next__(self):
return QuantileBufferIterator(self.elements, self.weighted, self.weight)
-class _QuantileState(object):
+class _QuantileState(Generic[T]):
"""
Compact summarization of a collection on which quantiles can be estimated.
"""
- min_val = None # Holds smallest item in the list
- max_val = None # Holds largest item in the list
+ min_val = None # type: Any # Holds smallest item in the list
Review comment:
T?
##########
File path: sdks/python/apache_beam/transforms/environments.py
##########
@@ -52,7 +55,7 @@
from apache_beam.utils import proto_utils
if TYPE_CHECKING:
- from apache_beam.options.pipeline_options import PortablePipelineOptions
+ from apache_beam.options.pipeline_options import PortableOptions
Review comment:
It'll be good to actually start enforcing these type checks so they
don't go out of date...
##########
File path: sdks/python/apache_beam/transforms/window.py
##########
@@ -164,14 +168,19 @@ def merge(self, merge_context):
raise NotImplementedError
def is_merging(self):
+ # type: () -> bool
+
"""Returns whether this WindowFn merges windows."""
return True
@abc.abstractmethod
def get_window_coder(self):
+ # type: () -> coders.Coder
Review comment:
This should be a `Coder[BoundedWindow]`. We could even make WindowFn
parameterized by the window type, and restrict this further.
##########
File path: sdks/python/apache_beam/transforms/combiners.py
##########
@@ -968,13 +968,13 @@ def expand(self, pcoll):
return (
pcoll
| core.ParDo(self.add_timestamp).with_output_types(
- Tuple[T, TimestampType]) # type: ignore[misc]
Review comment:
Nice? Does it cause any other issues?
##########
File path: sdks/python/apache_beam/transforms/util.py
##########
@@ -709,10 +710,10 @@ def expand(self, pcoll):
return (
pcoll
| 'AddRandomKeys' >> Map(lambda t: (random.getrandbits(32), t)).
- with_input_types(T).with_output_types(KeyedT) # type: ignore[misc]
+ with_input_types(T).with_output_types(KeyedT)
Review comment:
PythonFormatter needs another pass here.
----------------------------------------------------------------
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 495497)
Time Spent: 107h 50m (was: 107h 40m)
> Add type hints to python code
> -----------------------------
>
> Key: BEAM-7746
> URL: https://issues.apache.org/jira/browse/BEAM-7746
> Project: Beam
> Issue Type: New Feature
> Components: sdk-py-core
> Reporter: Chad Dombrova
> Priority: P3
> Time Spent: 107h 50m
> Remaining Estimate: 0h
>
> As a developer of the beam source code, I would like the code to use pep484
> type hints so that I can clearly see what types are required, get completion
> in my IDE, and enforce code correctness via a static analyzer like mypy.
> This may be considered a precursor to BEAM-7060
> Work has been started here: [https://github.com/apache/beam/pull/9056]
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
