PR comments

converters simpiliciation

Author: phi-dbq

python/sparkdl/param/converters.py

@@ -33,120 +33,131 @@ class SparkDLTypeConverters(object):
     These methods are similar to :py:class:`spark.ml.param.TypeConverters`.
     They provide support for the `Params` types introduced in Spark Deep Learning Pipelines.
     """
+
     @staticmethod
     def toTFGraph(value):
-        if isinstance(value, tf.Graph):
-            return value
-        else:
-            raise TypeError("Could not convert %s to TensorFlow Graph" % type(value))
+        if not isinstance(value, tf.Graph):
+            raise TypeError("Could not convert %s to tf.Graph" % type(value))
+        return value
 
     @staticmethod
     def asColumnToTensorNameMap(value):
         """
         Convert a value to a column name to :py:obj:`tf.Tensor` name mapping
         as a sorted list of string pairs, if possible.
         """
-        if isinstance(value, dict):
-            strs_pair_seq = []
-            for _maybe_col_name, _maybe_tnsr_name in value.items():
-                # Check if the non-tensor value is of string type
-                _col_name = _get_strict_col_name(_maybe_col_name)
-                # Check if the tensor name is actually valid
-                _tnsr_name = _get_strict_tensor_name(_maybe_tnsr_name)
-                strs_pair_seq.append((_col_name, _tnsr_name))
+        if not isinstance(value, dict):
+            err_msg = "Could not convert [type {}] {} to column name to tf.Tensor name mapping"
+            raise TypeError(err_msg.format(type(value), value))
 
-            return sorted(strs_pair_seq)
+        # Convertion logic after quick type check
+        strs_pair_seq = []
+        for _maybe_col_name, _maybe_tnsr_name in value.items():
+            # Check if the non-tensor value is of string type
+            _check_is_str(_maybe_col_name)
+            # Check if the tensor name looks like a tensor name
+            _check_is_tensor_name(_maybe_tnsr_name)
+            strs_pair_seq.append((_maybe_col_name, _maybe_tnsr_name))
 
-        err_msg = "Could not convert [type {}] {} to column name to tf.Tensor name mapping"
-        raise TypeError(err_msg.format(type(value), value))
+        return sorted(strs_pair_seq)
 
     @staticmethod
     def asTensorNameToColumnMap(value):
         """
         Convert a value to a :py:obj:`tf.Tensor` name to column name mapping
         as a sorted list of string pairs, if possible.
         """
-        if isinstance(value, dict):
-            strs_pair_seq = []
-            for _maybe_tnsr_name, _maybe_col_name in value.items():
-                # Check if the non-tensor value is of string type
-                _col_name = _get_strict_col_name(_maybe_col_name)
-                # Check if the tensor name is actually valid
-                _tnsr_name = _get_strict_tensor_name(_maybe_tnsr_name)
-                strs_pair_seq.append((_tnsr_name, _col_name))
+        if not isinstance(value, dict):
+            err_msg = "Could not convert [type {}] {} to tf.Tensor name to column name mapping"
+            raise TypeError(err_msg.format(type(value), value))
 
-            return sorted(strs_pair_seq)
+        # Convertion logic after quick type check
+        strs_pair_seq = []
+        for _maybe_tnsr_name, _maybe_col_name in value.items():
+            # Check if the non-tensor value is of string type
+            _check_is_str(_maybe_col_name)
+            # Check if the tensor name looks like a tensor name
+            _check_is_tensor_name(_maybe_tnsr_name)
+            strs_pair_seq.append((_maybe_tnsr_name, _maybe_col_name))
 
-        err_msg = "Could not convert [type {}] {} to tf.Tensor name to column name mapping"
-        raise TypeError(err_msg.format(type(value), value))
+        return sorted(strs_pair_seq)
 
     @staticmethod
     def toTFHParams(value):
         """ Convert a value to a :py:obj:`tf.contrib.training.HParams` object, if possible. """
-        if isinstance(value, tf.contrib.training.HParams):
-            return value
-        else:
+        if not isinstance(value, tf.contrib.training.HParams):
             raise TypeError("Could not convert %s to TensorFlow HParams" % type(value))
 
+        return value
+
     @staticmethod
-    def toStringOrTFTensor(value):
+    def toTFTensorName(value):
         """ Convert a value to a str or a :py:obj:`tf.Tensor` object, if possible. """
         if isinstance(value, tf.Tensor):
-            return value
+            return value.name
         try:
+            _check_is_tensor_name(value)
             return TypeConverters.toString(value)
         except Exception as exc:
             err_msg = "Could not convert [type {}] {} to tf.Tensor or str. {}"
             raise TypeError(err_msg.format(type(value), value, exc))
 
     @staticmethod
-    def supportedNameConverter(supportedList):
+    def buildCheckList(supportedList):
         """
         Create a converter that try to check if a value is part of the supported list.
 
         :param supportedList: list, containing supported objects.
         :return: a converter that try to convert a value if it is part of the `supportedList`.
         """
+
         def converter(value):
-            if value in supportedList:
-                return value
-            err_msg = "[type {}] {} is not in the supported list: {}"
-            raise TypeError(err_msg.format(type(value), str(value), supportedList))
+            if value not in supportedList:
+                err_msg = "[type {}] {} is not in the supported list: {}"
+                raise TypeError(err_msg.format(type(value), str(value), supportedList))
+
+            return value
 
         return converter
 
     @staticmethod
     def toKerasLoss(value):
         """ Convert a value to a name of Keras loss function, if possible """
-        if kmutil.is_valid_loss_function(value):
-            return value
-        err_msg = "Named loss not supported in Keras: [type {}] {}"
-        raise ValueError(err_msg.format(type(value), value))
+        # return early in for clarify as well as less indentation
+        if not kmutil.is_valid_loss_function(value):
+            err_msg = "Named loss not supported in Keras: [type {}] {}"
+            raise ValueError(err_msg.format(type(value), value))
+
+        return value
 
     @staticmethod
     def toKerasOptimizer(value):
         """ Convert a value to a name of Keras optimizer, if possible """
-        if kmutil.is_valid_optimizer(value):
-            return value
-        err_msg = "Named optimizer not supported in Keras: [type {}] {}"
-        raise TypeError(err_msg.format(type(value), value))
+        if not kmutil.is_valid_optimizer(value):
+            err_msg = "Named optimizer not supported in Keras: [type {}] {}"
+            raise TypeError(err_msg.format(type(value), value))
+
+        return value
 
 
-def _get_strict_tensor_name(_maybe_tnsr_name):
+def _check_is_tensor_name(_maybe_tnsr_name):
     """ Check if the input is a valid tensor name """
     try:
         assert isinstance(_maybe_tnsr_name, six.string_types), \
             "must provide a strict tensor name as input, but got {}".format(type(_maybe_tnsr_name))
-        assert tfx.as_tensor_name(_maybe_tnsr_name) == _maybe_tnsr_name, \
-            "input {} must be a valid tensor name".format(_maybe_tnsr_name)
+
+        # The check is taken from TensorFlow's NodeDef protocol buffer.
+        # https://github.com/tensorflow/tensorflow/blob/r1.3/tensorflow/core/framework/node_def.proto#L21-L25
+        _, src_idx = _maybe_tnsr_name.split(":")
+        _ = int(src_idx)
     except Exception as exc:
         err_msg = "Can NOT convert [type {}] {} to tf.Tensor name: {}"
         raise TypeError(err_msg.format(type(_maybe_tnsr_name), _maybe_tnsr_name, exc))
     else:
         return _maybe_tnsr_name
 
 
-def _get_strict_col_name(_maybe_col_name):
+def _check_is_str(_maybe_col_name):
     """ Check if the given colunm name is a valid column name """
     # We only check if the column name candidate is a string type
     if not isinstance(_maybe_col_name, six.string_types):

python/sparkdl/param/image_params.py

@@ -107,7 +107,7 @@ class HasOutputMode(Params):
                        "How the output column should be formatted. 'vector' for a 1-d MLlib " +
                        "Vector of floats. 'image' to format the output to work with the image " +
                        "tools in this package.",
-                       typeConverter=SparkDLTypeConverters.supportedNameConverter(OUTPUT_MODES))
+                       typeConverter=SparkDLTypeConverters.buildCheckList(OUTPUT_MODES))
 
     def setOutputMode(self, value):
         return self._set(outputMode=value)

python/sparkdl/transformers/named_image.py

@@ -40,7 +40,7 @@ class DeepImagePredictor(Transformer, HasInputCol, HasOutputCol):
     """
 
     modelName = Param(Params._dummy(), "modelName", "A deep learning model name",
-                      typeConverter=SparkDLTypeConverters.supportedNameConverter(SUPPORTED_MODELS))
+                      typeConverter=SparkDLTypeConverters.buildCheckList(SUPPORTED_MODELS))
     decodePredictions = Param(Params._dummy(), "decodePredictions",
                               "If true, output predictions in the (class, description, probability) format",
                               typeConverter=TypeConverters.toBoolean)

python/sparkdl/transformers/tf_image.py

@@ -54,10 +54,10 @@ class TFImageTransformer(Transformer, HasInputCol, HasOutputCol, HasOutputMode):
                   typeConverter=SparkDLTypeConverters.toTFGraph)
     inputTensor = Param(Params._dummy(), "inputTensor",
                         "A TensorFlow tensor object or name representing the input image",
-                        typeConverter=SparkDLTypeConverters.toStringOrTFTensor)
+                        typeConverter=SparkDLTypeConverters.toTFTensorName)
     outputTensor = Param(Params._dummy(), "outputTensor",
                          "A TensorFlow tensor object or name representing the output",
-                         typeConverter=SparkDLTypeConverters.toStringOrTFTensor)
+                         typeConverter=SparkDLTypeConverters.toTFTensorName)
 
     @keyword_only
     def __init__(self, inputCol=None, outputCol=None, graph=None,
@@ -99,18 +99,12 @@ def getGraph(self):
         return self.getOrDefault(self.graph)
 
     def getInputTensor(self):
-        tensor_or_name = self.getOrDefault(self.inputTensor)
-        if isinstance(tensor_or_name, tf.Tensor):
-            return tensor_or_name
-        else:
-            return self.getGraph().get_tensor_by_name(tensor_or_name)
+        tensor_name = self.getOrDefault(self.inputTensor)
+        return self.getGraph().get_tensor_by_name(tensor_name)
 
     def getOutputTensor(self):
-        tensor_or_name = self.getOrDefault(self.outputTensor)
-        if isinstance(tensor_or_name, tf.Tensor):
-            return tensor_or_name
-        else:
-            return self.getGraph().get_tensor_by_name(tensor_or_name)
+        tensor_name = self.getOrDefault(self.outputTensor)
+        return self.getGraph().get_tensor_by_name(tensor_name)
 
     def _transform(self, dataset):
         graph = self.getGraph()