Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sdks/python/apache_beam/typehints/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

"""A package defining the syntax and decorator semantics for type-hints."""

from __future__ import absolute_import

# pylint: disable=wildcard-import
from apache_beam.typehints.typehints import *
from apache_beam.typehints.decorators import *
9 changes: 7 additions & 2 deletions sdks/python/apache_beam/typehints/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,13 @@ def foo((a, b)):
defined, or before importing a module containing type-hinted functions.
"""

from __future__ import absolute_import

import inspect
import types
from builtins import next
from builtins import object
from builtins import zip

from apache_beam.typehints import native_type_compatibility
from apache_beam.typehints import typehints
Expand Down Expand Up @@ -175,7 +180,7 @@ def with_defaults(self, hints):
return IOTypeHints(self.input_types or hints.input_types,
self.output_types or hints.output_types)

def __nonzero__(self):
def __bool__(self):
return bool(self.input_types or self.output_types)

def __repr__(self):
Expand Down Expand Up @@ -404,7 +409,7 @@ def with_output_types(*return_type_hint, **kwargs):
from apache_beam.typehints import with_output_types
from apache_beam.typehints import Set

class Coordinate:
class Coordinate(object):
def __init__(self, x, y):
self.x = x
self.y = y
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@

"""Module to convert Python's native typing types to Beam types."""

from __future__ import absolute_import

import collections
import typing
from builtins import next
from builtins import range

from apache_beam.typehints import typehints

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

"""Test for Beam type compatibility library."""

from __future__ import absolute_import

import typing
import unittest

Expand Down
9 changes: 6 additions & 3 deletions sdks/python/apache_beam/typehints/opcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
import types
from functools import reduce

import six

from . import typehints
from .trivial_inference import BoundMethod
from .trivial_inference import Const
Expand All @@ -47,6 +45,11 @@
from .typehints import Tuple
from .typehints import Union

try: # Python 2
unicode # pylint: disable=unicode-builtin
except NameError: # Python 3
unicode = str


def pop_one(state, unused_arg):
del state.stack[-1:]
Expand Down Expand Up @@ -152,7 +155,7 @@ def binary_true_divide(state, unused_arg):
def binary_subscr(state, unused_arg):
index = state.stack.pop()
base = state.stack.pop()
if base in (str, six.text_type):
if base in (str, unicode):
out = base
elif (isinstance(index, Const) and isinstance(index.value, int)
and isinstance(base, typehints.TupleHint.TupleConstraint)):
Expand Down
13 changes: 12 additions & 1 deletion sdks/python/apache_beam/typehints/trivial_inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,19 @@
import pprint
import sys
import types
from builtins import object
from builtins import zip
from functools import reduce

from apache_beam.typehints import Any
from apache_beam.typehints import typehints
from six.moves import builtins

# pylint: disable=wrong-import-order, wrong-import-position, ungrouped-imports
try: # Python 2
import __builtin__ as builtins
except ImportError: # Python 3
import builtins
# pylint: enable=wrong-import-order, wrong-import-position, ungrouped-imports


class TypeInferenceError(ValueError):
Expand Down Expand Up @@ -115,6 +123,9 @@ def __init__(self, f, local_vars=None, stack=()):
def __eq__(self, other):
return isinstance(other, FrameState) and self.__dict__ == other.__dict__

def __hash__(self):
return hash(tuple(sorted(self.__dict__.items())))

def copy(self):
return FrameState(self.f, self.vars, self.stack)

Expand Down
3 changes: 3 additions & 0 deletions sdks/python/apache_beam/typehints/trivial_inference_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
#

"""Tests for apache_beam.typehints.trivial_inference."""

from __future__ import absolute_import

import unittest

from apache_beam.typehints import trivial_inference
Expand Down
21 changes: 13 additions & 8 deletions sdks/python/apache_beam/typehints/typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
For internal use only; no backwards-compatibility guarantees.
"""

from __future__ import absolute_import

import collections
import inspect
import sys
import types

import six
from future.utils import raise_with_traceback

from apache_beam import pipeline
from apache_beam.pvalue import TaggedOutput
Expand All @@ -41,6 +41,11 @@
from apache_beam.typehints.typehints import SimpleTypeHintError
from apache_beam.typehints.typehints import check_constraint

try: # Python 2
unicode # pylint: disable=unicode-builtin
except NameError: # Python 3
unicode = str


class AbstractDoFnWrapper(DoFn):
"""An abstract class to create wrapper around DoFn"""
Expand Down Expand Up @@ -87,14 +92,14 @@ def wrapper(self, method, args, kwargs):
except TypeCheckError as e:
error_msg = ('Runtime type violation detected within ParDo(%s): '
'%s' % (self.full_label, e))
six.raise_from(TypeCheckError(error_msg), sys.exc_info()[2])
raise_with_traceback(TypeCheckError(error_msg))
else:
return self._check_type(result)

def _check_type(self, output):
if output is None:
return output
elif isinstance(output, (dict,) + six.string_types):
elif isinstance(output, (dict, bytes, str, unicode)):
object_type = type(output).__name__
raise TypeCheckError('Returning a %s from a ParDo or FlatMap is '
'discouraged. Please use list("%s") if you really '
Expand Down Expand Up @@ -176,12 +181,12 @@ def _type_check(self, type_constraint, datum, is_input):
try:
check_constraint(type_constraint, datum)
except CompositeTypeHintError as e:
six.raise_from(TypeCheckError(e.args[0]), sys.exc_info()[2])
raise_with_traceback(TypeCheckError(e.args[0]))
except SimpleTypeHintError:
error_msg = ("According to type-hint expected %s should be of type %s. "
"Instead, received '%s', an instance of type %s."
% (datum_type, type_constraint, datum, type(datum)))
six.raise_from(TypeCheckError(error_msg), sys.exc_info()[2])
raise_with_traceback(TypeCheckError(error_msg))


class TypeCheckCombineFn(core.CombineFn):
Expand All @@ -206,7 +211,7 @@ def add_input(self, accumulator, element, *args, **kwargs):
except TypeCheckError as e:
error_msg = ('Runtime type violation detected within %s: '
'%s' % (self._label, e))
six.raise_from(TypeCheckError(error_msg), sys.exc_info()[2])
raise_with_traceback(TypeCheckError(error_msg))
return self._combinefn.add_input(accumulator, element, *args, **kwargs)

def merge_accumulators(self, accumulators, *args, **kwargs):
Expand All @@ -221,7 +226,7 @@ def extract_output(self, accumulator, *args, **kwargs):
except TypeCheckError as e:
error_msg = ('Runtime type violation detected within %s: '
'%s' % (self._label, e))
six.raise_from(TypeCheckError(error_msg), sys.exc_info()[2])
raise_with_traceback(TypeCheckError(error_msg))
return result


Expand Down
3 changes: 3 additions & 0 deletions sdks/python/apache_beam/typehints/typed_pipeline_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
#

"""Unit tests for the type-hint objects and decorators."""

from __future__ import absolute_import

import inspect
import typing
import unittest
Expand Down
22 changes: 17 additions & 5 deletions sdks/python/apache_beam/typehints/typehints.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,16 @@

"""

from __future__ import absolute_import

import collections
import copy
import sys
import types
from builtins import next
from builtins import zip

import six
from future.utils import with_metaclass

__all__ = [
'Any',
Expand Down Expand Up @@ -411,17 +415,25 @@ def __eq__(self, other):
def __repr__(self):
return 'Any'

def __hash__(self):
# TODO(BEAM - 3730)
return hash(id(self))

def type_check(self, instance):
pass


class TypeVariable(AnyTypeConstraint):

def __init__(self, name):
self.name = name

def __eq__(self, other):
return type(self) == type(other) and self.name == other.name

def __init__(self, name):
self.name = name
def __hash__(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change the implementation hash(self.name) to fulfilll the contract between __hash__ and __eq__.

Copy link
Contributor Author

@RobbeSneyders RobbeSneyders Jun 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is again the same issue as mentioned above.

I actually think it might make more sense to change __eq__ to be instance specific. If I specify K = beam.typehints.TypeVariable('K') in two separate places in my code base, I might not want them match.
However, this might break some existing pipelines. I therefore tried to use the equivalent of the Python 2 code.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reproduced the BEAM-3730 but could not easily interpret the failure mode. It is clear that equality relationship is not defined correctly here. I'd like to understand the failure first to understand what assumptions Beam codebase has about typehints. I don't have a good answer right now. For the purpose of this change, I think keeping hash(id(self)) is ok for now since we won't make things more broken with that, but we should add a TODO(BEAM-3730): Fix the contract between __hash__ and eq. I'll try to add more context to the issue later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a TODO(BEAM-3730) comment here too?

# TODO(BEAM - 3730)
return hash(id(self))

def __repr__(self):
return 'TypeVariable[%s]' % self.name
Expand Down Expand Up @@ -992,8 +1004,8 @@ def __getitem__(self, type_param):
IteratorTypeConstraint = IteratorHint.IteratorTypeConstraint


@six.add_metaclass(GetitemConstructor)
class WindowedTypeConstraint(TypeConstraint):
class WindowedTypeConstraint(with_metaclass(GetitemConstructor,
TypeConstraint)):
"""A type constraint for WindowedValue objects.

Mostly for internal use.
Expand Down
5 changes: 5 additions & 0 deletions sdks/python/apache_beam/typehints/typehints_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@
#

"""Unit tests for the type-hint objects and decorators."""

from __future__ import absolute_import

import functools
import inspect
import unittest
from builtins import next
from builtins import range

import apache_beam.typehints.typehints as typehints
from apache_beam.typehints import Any
Expand Down
1 change: 1 addition & 0 deletions sdks/python/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ modules =
apache_beam/pvalue_test
apache_beam/testing
apache_beam/tools
apache_beam/typehints
commands =
python --version
pip --version
Expand Down