PR fixes, among them: bring back the documentation for deprecated modelset and modelrun. Validate that all required arguments appear in the new commands (+test it)

alonre24 · alonre24 · commit e91fce51b6b6 · 2021-05-19T14:21:02.000+03:00
diff --git a/redisai/client.py b/redisai/client.py
@@ -212,7 +212,7 @@ def modelstore(
         res = self.execute_command(*args)
         return res if not self.enable_postprocess else processor.modelstore(res)
 
-    @deprecated(version="1.2.2", reason="Use modelstore instead")
+    @deprecated(version="1.2.0", reason="Use modelstore instead")
     def modelset(
         self,
         key: AnyStr,
@@ -226,8 +226,34 @@ def modelset(
         outputs: Union[AnyStr, List[AnyStr]] = None,
     ) -> str:
         """
-        Similar to modelstore (this is the deprecated version that will not be
-        supported in future versions).
+        Set the model on provided key.
+
+        Parameters
+        ----------
+        key : AnyStr
+            Key name
+        backend : str
+            Backend name. Allowed backends are TF, TORCH, TFLITE, ONNX
+        device : str
+            Device name. Allowed devices are CPU and GPU. If multiple GPUs are available,
+            it can be specified using the format GPU:<gpu number>. For example: GPU:0
+        data : bytes
+            Model graph read as bytes string
+        batch : int
+            Number of batches for doing auto-batching
+        minbatch : int
+            Minimum number of samples required in a batch for model execution
+        tag : AnyStr
+            Any string that will be saved in RedisAI as tag for the model
+        inputs : Union[AnyStr, List[AnyStr]]
+            Input node(s) in the graph. Required only Tensorflow graphs
+        outputs : Union[AnyStr, List[AnyStr]]
+            Output node(s) in the graph Required only for Tensorflow graphs
+
+        Returns
+        -------
+        str
+            'OK' if success, raise an exception otherwise
 
         Example
         -------
@@ -347,16 +373,34 @@ def modelexecute(
         res = self.execute_command(*args)
         return res if not self.enable_postprocess else processor.modelexecute(res)
 
-    @deprecated(version="1.2.2", reason="Use modelexecute instead")
+    @deprecated(version="1.2.0", reason="Use modelexecute instead")
     def modelrun(
         self,
         key: AnyStr,
         inputs: Union[AnyStr, List[AnyStr]],
         outputs: Union[AnyStr, List[AnyStr]],
     ) -> str:
         """
-        Similar to modelexecute (this is the deprecated version that will not be
-        supported in future versions).
+        Run the model using input(s) which are already in the scope and are associated
+        to some keys. Modelrun also needs the output key name(s) to store the output
+        from the model. The number of outputs from the model and the number of keys
+        provided here must be same. Otherwise, RedisAI throws an error
+
+        Parameters
+        ----------
+        key : str
+            Model key to run
+        inputs : Union[AnyStr, List[AnyStr]]
+            Tensor(s) which is already saved in the RedisAI using a tensorset call. These
+            tensors will be used as the input for the modelrun
+        outputs : Union[AnyStr, List[AnyStr]]
+            keys on which the outputs to be saved. If those keys exist already, modelrun
+            will overwrite them with new values
+
+        Returns
+        -------
+        str
+            'OK' if success, raise an exception otherwise
 
         Example
         -------
diff --git a/redisai/command_builder.py b/redisai/command_builder.py
@@ -23,12 +23,12 @@ def modelstore(
     inputs: Union[AnyStr, List[AnyStr]],
     outputs: Union[AnyStr, List[AnyStr]],
 ) -> Sequence:
+    if name is None:
+        raise ValueError("Model name was not given")
     if device.upper() not in utils.allowed_devices:
-        raise ValueError(
-            f"Device not allowed. Use any from {utils.allowed_devices}")
+        raise ValueError(f"Device not allowed. Use any from {utils.allowed_devices}")
     if backend.upper() not in utils.allowed_backends:
-        raise ValueError(
-            f"Backend not allowed. Use any from {utils.allowed_backends}")
+        raise ValueError(f"Backend not allowed. Use any from {utils.allowed_backends}")
     args = ["AI.MODELSTORE", name, backend, device]
 
     if tag is not None:
@@ -64,8 +64,7 @@ def modelstore(
             "Inputs and outputs keywords should not be specified for this backend"
         )
     chunk_size = 500 * 1024 * 1024  # TODO: this should be configurable.
-    data_chunks = [data[i: i + chunk_size]
-                   for i in range(0, len(data), chunk_size)]
+    data_chunks = [data[i : i + chunk_size] for i in range(0, len(data), chunk_size)]
     # TODO: need a test case for this
     args += ["BLOB", *data_chunks]
     return args
@@ -83,11 +82,9 @@ def modelset(
     outputs: Union[AnyStr, List[AnyStr]],
 ) -> Sequence:
     if device.upper() not in utils.allowed_devices:
-        raise ValueError(
-            f"Device not allowed. Use any from {utils.allowed_devices}")
+        raise ValueError(f"Device not allowed. Use any from {utils.allowed_devices}")
     if backend.upper() not in utils.allowed_backends:
-        raise ValueError(
-            f"Backend not allowed. Use any from {utils.allowed_backends}")
+        raise ValueError(f"Backend not allowed. Use any from {utils.allowed_backends}")
     args = ["AI.MODELSET", name, backend, device]
 
     if tag is not None:
@@ -101,13 +98,11 @@ def modelset(
 
     if backend.upper() == "TF":
         if not (all((inputs, outputs))):
-            raise ValueError(
-                "Require keyword arguments input and output for TF models")
+            raise ValueError("Require keyword arguments input and output for TF models")
         args += ["INPUTS", *utils.listify(inputs)]
         args += ["OUTPUTS", *utils.listify(outputs)]
     chunk_size = 500 * 1024 * 1024
-    data_chunks = [data[i: i + chunk_size]
-                   for i in range(0, len(data), chunk_size)]
+    data_chunks = [data[i : i + chunk_size] for i in range(0, len(data), chunk_size)]
     # TODO: need a test case for this
     args += ["BLOB", *data_chunks]
     return args
@@ -130,6 +125,8 @@ def modelexecute(
     outputs: Union[AnyStr, List[AnyStr]],
     timeout: int,
 ) -> Sequence:
+    if name is None or inputs is None or outputs is None:
+        raise ValueError("Missing required arguments for model execute command")
     args = [
         "AI.MODELEXECUTE",
         name,
@@ -209,8 +206,7 @@ def tensorget(key: AnyStr, as_numpy: bool = True, meta_only: bool = False) -> Se
 
 def scriptset(name: AnyStr, device: str, script: str, tag: AnyStr = None) -> Sequence:
     if device.upper() not in utils.allowed_devices:
-        raise ValueError(
-            f"Device not allowed. Use any from {utils.allowed_devices}")
+        raise ValueError(f"Device not allowed. Use any from {utils.allowed_devices}")
     args = ["AI.SCRIPTSET", name, device]
     if tag:
         args += ["TAG", tag]
diff --git a/redisai/postprocessor.py b/redisai/postprocessor.py
@@ -42,8 +42,7 @@ def tensorget(res, as_numpy, as_numpy_mutable, meta_only):
                 mutable=False,
             )
         else:
-            target = float if rai_result["dtype"] in (
-                "FLOAT", "DOUBLE") else int
+            target = float if rai_result["dtype"] in ("FLOAT", "DOUBLE") else int
             utils.recursive_bytetransform(rai_result["values"], target)
             return rai_result
 
diff --git a/redisai/utils.py b/redisai/utils.py
@@ -26,8 +26,7 @@ def numpy2blob(tensor: np.ndarray) -> tuple:
     try:
         dtype = dtype_dict[str(tensor.dtype)]
     except KeyError:
-        raise TypeError(
-            f"RedisAI doesn't support tensors of type {tensor.dtype}")
+        raise TypeError(f"RedisAI doesn't support tensors of type {tensor.dtype}")
     shape = tensor.shape
     blob = bytes(tensor.data)
     return dtype, shape, blob
diff --git a/setup.py b/setup.py
@@ -6,15 +6,15 @@
 
 setup(
     name="redisai",
-    version="1.0.2",
+    version="1.2.0",
     description="RedisAI Python Client",
     long_description=long_description,
     long_description_content_type="text/x-rst",
     url="http://github.com/RedisAI/redisai-py",
     author="RedisLabs",
     author_email="oss@redislabs.com",
     packages=find_packages(),
-    install_requires=["redis", "hiredis", "numpy"],
+    install_requires=["redis", "hiredis", "numpy", "deprecated"],
     python_requires=">=3.6",
     classifiers=[
         "Development Status :: 4 - Beta",
diff --git a/test/test.py b/test/test.py
@@ -162,6 +162,18 @@ def test_modelstore_errors(self):
         model_path = os.path.join(MODEL_DIR, "graph.pb")
         model_pb = load_model(model_path)
         con = self.get_client()
+
+        with self.assertRaises(ValueError) as e:
+            con.modelstore(
+                None,
+                "TF",
+                "CPU",
+                model_pb,
+                inputs=["a", "b"],
+                outputs=["mul"]
+            )
+        self.assertEqual(str(e.exception), "Model name was not given")
+
         with self.assertRaises(ValueError) as e:
             con.modelstore(
                 "m",
@@ -304,6 +316,15 @@ def test_run_tf_model(self):
             "m", "tf", "cpu", model_pb, inputs=["a", "b"], outputs="mul", tag="v1.0"
         )
 
+        # Required arguments ar None
+        with self.assertRaises(ValueError) as e:
+            con.modelexecute(
+                "m",
+                inputs=None,
+                outputs=None
+            )
+        self.assertEqual(str(e.exception), "Missing required arguments for model execute command")
+
         # wrong model
         with self.assertRaises(ResponseError) as e:
             con.modelstore(

Original file line number	Diff line number	Diff line change
`@@ -42,8 +42,7 @@ def tensorget(res, as_numpy, as_numpy_mutable, meta_only):`
`42`	`42`	`mutable=False,`
`43`	`43`	`)`
`44`	`44`	`else:`
`45`		`- target = float if rai_result["dtype"] in (`
`46`		`- "FLOAT", "DOUBLE") else int`
	`45`	`+ target = float if rai_result["dtype"] in ("FLOAT", "DOUBLE") else int`
`47`	`46`	`utils.recursive_bytetransform(rai_result["values"], target)`
`48`	`47`	`return rai_result`
`49`	`48`