register python exceptions and update exceptions handling#3169
register python exceptions and update exceptions handling#3169askhade merged 1 commit intoonnx:masterfrom
Conversation
jcwchen
left a comment
There was a problem hiding this comment.
Others LGTM. Thank you so much for organizing exceptions.
| // Submodule `shape_inference` | ||
| auto shape_inference = onnx_cpp2py_export.def_submodule("shape_inference"); | ||
| shape_inference.doc() = "Shape Inference submodule"; | ||
| py::register_exception<InferenceError>(shape_inference, "InferenceError"); |
There was a problem hiding this comment.
Is this intended to cover both shape-inference errors as well as type-errors? Wondering why module is called "shape inference", while the exception is plain "InferenceError".
There was a problem hiding this comment.
yes both module and exception class are intended to handle both type and shape inference.... I am simply registering the exception here...
There was a problem hiding this comment.
I wonder if making strict_mode into an integer (called mode, restricted to values 0 and 1 currently) will give us more flexibility in the future. For example, may be we could treat it as a compatibility_mode indicator indicating that we desire the behavior in version 1.7 or 1.8, etc. In the future, we could potentially end up with more than 2 modes from a compatibility-mode perspective.
There was a problem hiding this comment.
I am leaning towards still keeping the strict_mode as true\false in python and change the one in c++ interface to int... when we add more granularity we can still keep the python api for backward compatibility and add a new api in python for the new approach
0268ade to
9bb3b88
Compare
Signed-off-by: Ashwini Khade <askhade@microsoft.com>
9bb3b88 to
9acb76a
Compare
Signed-off-by: Ashwini Khade askhade@microsoft.com