From 7e5d6c25f4ce9b97166902c79b5d40c791dc119c Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 18 Feb 2019 00:25:24 +0000 Subject: [PATCH 1/4] Add a better error message when unsupported ops are used for Dimension This fix is related to 25790. In 25790, `Dimension` with division will thrown an TypeError: ``` output_dim = input_shape[-1] / 2 ... TypeError: unsupported operand type(s) for /: 'Dimension' and 'int' ``` The error is expected, as Dimension only support `//` (not `/`). However, for backward compatible reasons, `__div__`(`/`) is actually implemented (but deprecated). See code. This issue causes confusion and it has been mentioned multiple times in GitHub and StackOverflow(https://stackoverflow.com/questions/51692253/typeerror-unsupported-operand-types-for-dimension-and-int) This fix is an attempt to print a better error message with: ``` TypeError: unsupported operand type(s) for /: 'Dimension' and 'int', please use // instead ``` Note that this PR does not change any current behavior, and have no impact with respect to backward-compatible `__div__`. It merely adds additional notes in the error message. Signed-off-by: Yong Tang --- tensorflow/python/framework/tensor_shape.py | 48 +++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tensorflow/python/framework/tensor_shape.py b/tensorflow/python/framework/tensor_shape.py index 40fccc86a34..0dc3dde4f6e 100644 --- a/tensorflow/python/framework/tensor_shape.py +++ b/tensorflow/python/framework/tensor_shape.py @@ -468,6 +468,54 @@ class Dimension(object): """ return self // other + def __rdiv__(self, other): + """Use `__floordiv__` via `x // y` instead. + + This function exists only to have a better error message. Instead of: + `TypeError: unsupported operand type(s) for /: 'int' and 'Dimension'`, + this function will explicitly call for usage of `//` instead. + + Args: + other: Another `Dimension`. + + Raises: + TypeError. + """ + raise TypeError("unsupported operand type(s) for /: '{}' and 'Dimension', " + "please use // instead".format(type(other).__name__)) + + def __truediv__(self, other): + """Use `__floordiv__` via `x // y` instead. + + This function exists only to have a better error message. Instead of: + `TypeError: unsupported operand type(s) for /: 'Dimension' and 'int'`, + this function will explicitly call for usage of `//` instead. + + Args: + other: Another `Dimension`. + + Raises: + TypeError. + """ + raise TypeError("unsupported operand type(s) for /: 'Dimension' and '{}', " + "please use // instead".format(type(other).__name__)) + + def __rtruediv__(self, other): + """Use `__floordiv__` via `x // y` instead. + + This function exists only to have a better error message. Instead of: + `TypeError: unsupported operand type(s) for /: 'int' and 'Dimension'`, + this function will explicitly call for usage of `//` instead. + + Args: + other: Another `Dimension`. + + Raises: + TypeError. + """ + raise TypeError("unsupported operand type(s) for /: '{}' and 'Dimension', " + "please use // instead".format(type(other).__name__)) + def __mod__(self, other): """Returns `self` modulo `other`. From 1c241df19ffbaf562ea51d67bb167245dd6b7b95 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 18 Feb 2019 00:36:06 +0000 Subject: [PATCH 2/4] Add test case for error message of Dimension div (/) Signed-off-by: Yong Tang --- .../python/framework/tensor_shape_test.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tensorflow/python/framework/tensor_shape_test.py b/tensorflow/python/framework/tensor_shape_test.py index 7d85e0a99e6..786e4ef762a 100644 --- a/tensorflow/python/framework/tensor_shape_test.py +++ b/tensorflow/python/framework/tensor_shape_test.py @@ -205,6 +205,23 @@ class DimensionTest(test_util.TensorFlowTestCase): reconstructed = ctor(*args) self.assertEquals(reconstructed, dim) + def testDiv(self): + # Note: This test is related to GitHub issue 25790. + six = tensor_shape.Dimension(6) + two = tensor_shape.Dimension(2) + message = (r"unsupported operand type\(s\) for \/: " + r"'Dimension' and 'Dimension', please use \/\/ instead") + with self.assertRaisesRegexp(TypeError, message): + _ = six / two + message = (r"unsupported operand type\(s\) for \/: " + r"'Dimension' and 'int', please use \/\/ instead") + with self.assertRaisesRegexp(TypeError, message): + _ = six / 2 + message = (r"unsupported operand type\(s\) for \/: " + r"'int' and 'Dimension', please use \/\/ instead") + with self.assertRaisesRegexp(TypeError, message): + _ = 6 / two + class ShapeTest(test_util.TensorFlowTestCase): From 12eadc6a00a81e68989b0c4bb5b5ab464024b007 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 18 Feb 2019 00:37:00 +0000 Subject: [PATCH 3/4] Add test case for __rdiv__ in python 2 Signed-off-by: Yong Tang --- tensorflow/python/framework/tensor_shape_div_test.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tensorflow/python/framework/tensor_shape_div_test.py b/tensorflow/python/framework/tensor_shape_div_test.py index 8e63d7f5470..a4422c9b18a 100644 --- a/tensorflow/python/framework/tensor_shape_div_test.py +++ b/tensorflow/python/framework/tensor_shape_div_test.py @@ -35,6 +35,15 @@ class DimensionDivTest(test_util.TensorFlowTestCase): for y in values: self.assertEqual((x / y).value, (x // y).value) + def testRDivFail(self): + # Note: This test is related to GitHub issue 25790. + """Without from __future__ import division, __rdiv__ is used.""" + if six.PY2: # Old division exists only in Python 2 + two = tensor_shape.Dimension(2) + message = (r"unsupported operand type\(s\) for \/: " + r"'int' and 'Dimension', please use \/\/ instead") + with self.assertRaisesRegexp(TypeError, message): + _ = 6 / two if __name__ == "__main__": googletest.main() From 8f0570b9627f12fc95b02eca70e6267735f9c717 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Tue, 19 Feb 2019 19:23:33 +0000 Subject: [PATCH 4/4] Remove unneeded quote for forward slash based on review comment. Signed-off-by: Yong Tang --- tensorflow/python/framework/tensor_shape_div_test.py | 4 ++-- tensorflow/python/framework/tensor_shape_test.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tensorflow/python/framework/tensor_shape_div_test.py b/tensorflow/python/framework/tensor_shape_div_test.py index a4422c9b18a..621471cadba 100644 --- a/tensorflow/python/framework/tensor_shape_div_test.py +++ b/tensorflow/python/framework/tensor_shape_div_test.py @@ -40,8 +40,8 @@ class DimensionDivTest(test_util.TensorFlowTestCase): """Without from __future__ import division, __rdiv__ is used.""" if six.PY2: # Old division exists only in Python 2 two = tensor_shape.Dimension(2) - message = (r"unsupported operand type\(s\) for \/: " - r"'int' and 'Dimension', please use \/\/ instead") + message = (r"unsupported operand type\(s\) for /: " + r"'int' and 'Dimension', please use // instead") with self.assertRaisesRegexp(TypeError, message): _ = 6 / two diff --git a/tensorflow/python/framework/tensor_shape_test.py b/tensorflow/python/framework/tensor_shape_test.py index 786e4ef762a..b4a37c05a83 100644 --- a/tensorflow/python/framework/tensor_shape_test.py +++ b/tensorflow/python/framework/tensor_shape_test.py @@ -209,16 +209,16 @@ class DimensionTest(test_util.TensorFlowTestCase): # Note: This test is related to GitHub issue 25790. six = tensor_shape.Dimension(6) two = tensor_shape.Dimension(2) - message = (r"unsupported operand type\(s\) for \/: " - r"'Dimension' and 'Dimension', please use \/\/ instead") + message = (r"unsupported operand type\(s\) for /: " + r"'Dimension' and 'Dimension', please use // instead") with self.assertRaisesRegexp(TypeError, message): _ = six / two - message = (r"unsupported operand type\(s\) for \/: " - r"'Dimension' and 'int', please use \/\/ instead") + message = (r"unsupported operand type\(s\) for /: " + r"'Dimension' and 'int', please use // instead") with self.assertRaisesRegexp(TypeError, message): _ = six / 2 - message = (r"unsupported operand type\(s\) for \/: " - r"'int' and 'Dimension', please use \/\/ instead") + message = (r"unsupported operand type\(s\) for /: " + r"'int' and 'Dimension', please use // instead") with self.assertRaisesRegexp(TypeError, message): _ = 6 / two