Fixes #233, improve error handling (#242)

* Fixes #233 -- adds details to error handling, raises error when model is invalid

* consistent onnx model checks, error messages

* Removing try+exception blocks so if it fails, it will throw original exception

Author: vinitra

onnxmltools/utils/main.py

@@ -27,16 +27,8 @@ def load_model(file_path):
     if not path.exists(file_path):
         raise FileNotFoundError("{0} was not found.".format(file_path))
     model = onnx_proto.ModelProto()
-    try:
-        with open(file_path, 'rb') as f:
-            model.ParseFromString(f.read())
-    except IOError:
-        print("File '{0}' could not be opened.".format(file_path))
-        return
-    except:
-        print("Could not load protobuf file.")
-        return
-
+    with open(file_path, 'rb') as f:
+        model.ParseFromString(f.read())
     return model
 
 
@@ -53,18 +45,13 @@ def save_model(model, file_path):
         from onnxmltools.utils import save_model
         save_model(onnx_model, 'c:/test_model.onnx')
     """
+    if model is None or not isinstance(model, onnx_proto.ModelProto):
+        raise ValueError("Model is not a valid ONNX model.")
     directory = os.path.dirname(os.path.abspath(file_path))
     if not path.exists(directory):
         raise FileNotFoundError("Directory does not exist {0}".format(directory))
-    try:
-        with open(file_path, 'wb') as f:
-            f.write(model.SerializeToString())
-    except IOError:
-        print("Unable to write file to path '{0}', check if you have permissions.".format(file_path))
-        return
-    except:
-        print("Failed trying to save file '{0}'.".format(file_path))
-        return
+    with open(file_path, 'wb') as f:
+        f.write(model.SerializeToString())
 
 
 def save_text(model, file_path):
@@ -81,11 +68,10 @@ def save_text(model, file_path):
         from onnxmltools.utils import save_text
         save_text(onnx_model,"SqueezeNet.json")
     """
-    try:
-        with open(file_path, "w") as f:
-            f.write(str(model))
-    except IOError:
-        print("Could not save file")
+    if model is None or not isinstance(model, onnx_proto.ModelProto):
+        raise ValueError("Model is not a valid ONNX model.")
+    with open(file_path, "w") as f:
+        f.write(str(model))
 
 
 def set_model_domain(model, domain):
@@ -103,9 +89,9 @@ def set_model_domain(model, domain):
         set_model_domain(onnx_model, "com.acme")
     """
     if model is None or not isinstance(model, onnx_proto.ModelProto):
-        raise ValueError("model is not an onnx model")
+        raise ValueError("Model is not a valid ONNX model.")
     if not convert_utils.is_string_type(domain):
-        raise ValueError("domain must be a string type")
+        raise ValueError("Domain must be a string type.")
     model.domain = domain
 
 
@@ -124,9 +110,9 @@ def set_model_version(model, version):
         set_model_version(onnx_model, 1)
     """
     if model is None or not isinstance(model, onnx_proto.ModelProto):
-        raise ValueError("model is not an onnx model")
+        raise ValueError("Model is not a valid ONNX model.")
     if not convert_utils.is_numeric_type(version):
-        raise ValueError("version must be a numeric type")
+        raise ValueError("Version must be a numeric type.")
     model.model_version = version
 
 
@@ -146,9 +132,9 @@ def set_model_doc_string(model, doc, override=False):
         set_model_doc_string(onnx_model, "Sample doc string")
     """
     if model is None or not isinstance(model, onnx_proto.ModelProto):
-        raise ValueError("model is not an onnx model")
+        raise ValueError("Model is not a valid ONNX model.")
     if not convert_utils.is_string_type(doc):
-        raise ValueError("doc must be a string type")
+        raise ValueError("Doc must be a string type.")
     if model.doc_string and not doc and override is False:
-        raise ValueError("failing to overwrite the doc string with a blank string, set override to True if intentional")
+        raise ValueError("Failing to overwrite the doc string with a blank string, set override to True if intentional.")
     model.doc_string = doc