tvalentyn commented on a change in pull request #14634:
URL: https://github.com/apache/beam/pull/14634#discussion_r622719373
##########
File path: sdks/python/apache_beam/transforms/combiners.py
##########
@@ -178,72 +176,30 @@ class Of(CombinerWithoutDefaults):
to which it is applied, where "greatest" is determined by the comparator
function supplied as the compare argument.
"""
- def _py2__init__(self, n, compare=None, *args, **kwargs):
- """Initializer.
-
- compare should be an implementation of "a < b" taking at least two
- arguments (a and b). Additional arguments and side inputs specified in
- the apply call become additional arguments to the comparator. Defaults to
- the natural ordering of the elements.
- The arguments 'key' and 'reverse' may instead be passed as keyword
- arguments, and have the same meaning as for Python's sort functions.
-
- Args:
- pcoll: PCollection to process.
- n: number of elements to extract from pcoll.
- compare: as described above.
- *args: as described above.
- **kwargs: as described above.
- """
- super(Top.Of, self).__init__()
- if compare:
- warnings.warn(
- 'Compare not available in Python 3, use key instead.',
- DeprecationWarning)
- self._n = n
- self._compare = compare
- self._key = kwargs.pop('key', None)
- self._reverse = kwargs.pop('reverse', False)
- self._args = args
- self._kwargs = kwargs
-
- def _py3__init__(self, n, **kwargs):
+ def __init__(self, n, key=None, reverse=False):
"""Creates a global Top operation.
The arguments 'key' and 'reverse' may be passed as keyword arguments,
and have the same meaning as for Python's sort functions.
Args:
- pcoll: PCollection to process.
n: number of elements to extract from pcoll.
- **kwargs: may contain 'key' and/or 'reverse'
+ key: (optional) a mapping of elements to a comparable key, similar to
+ the key argument of Python's sorting methods.
+ reverse: (optional) whether to order things smallest to largest, rather
+ than largest to smallest
"""
- unknown_kwargs = set(kwargs.keys()) - set(['key', 'reverse'])
- if unknown_kwargs:
- raise ValueError(
- 'Unknown keyword arguments: ' + ', '.join(unknown_kwargs))
- self._py2__init__(n, None, **kwargs)
-
- # Python 3 sort does not accept a comparison operator, and nor do we.
- # FIXME: mypy would handle this better if we placed the _py*__init__ funcs
- # inside the if/else block below:
- if sys.version_info[0] < 3:
- __init__ = _py2__init__
- else:
- __init__ = _py3__init__ # type: ignore
+ super(Top.Of, self).__init__()
+ self._n = n
+ self._key = key
+ self._reverse = reverse
def default_label(self):
return 'Top(%d)' % self._n
def expand(self, pcoll):
- compare = self._compare
- if (not self._args and not self._kwargs and
pcoll.windowing.is_default()):
- if self._reverse:
- if compare is None or compare is operator.lt:
- compare = operator.gt
- else:
- original_compare = compare
- compare = lambda a, b: original_compare(b, a)
+ if pcoll.windowing.is_default():
+ compare = operator.gt if self._reverse else None
Review comment:
looks like this is not necessary due to line 301 .
--
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]
