-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Original discussion #116
@RobPasMue what's the rationale behind creating our own exception hierarchy (specifically, the use of
ProductInstanceErrorwhere it was a plainRuntimeErrorbefore) rather than using the built-in exceptions? This is causing some problems in PyACP since I'm now catching the wrong exception type. It also means the uncaught exceptions will change type, which is technically a backwards-incompatible change to PyACP.As such, I'm trying to figure out if PyACP should go the same route, or spend extra effort to hide this change.
That's a good point @greschd - the goal was to provide a set of common errors related to Ansys libraries that we could all extend from and make the experience for our users more streamlined -- they could simply catch our error and make sure that everything related to Ansys can be captured by it.. nonetheless, up for discussion. We haven't enforced it anywhere yet.
@RobPasMue I think the custom exceptions can make sense when there's scenario that's somewhat specific to our libraries. For example
VersionError, or also theProductInstanceError.I'm less convinced that errors which basically mirror built-in exceptions make sense (
AnsysTypeError,AnsysLogicError). Unless we want to mirror the entire exception hierarchy, we will end up with overly broad exceptions.they could simply catch our error and make sure that everything related to Ansys can be captured by it.. nonetheless, up for discussion
Doesn't that promise indirectly imply that we'd write exception-free code / catch and re-raise all exceptions? I don't think it makes sense to raise an
AnsysTypeErrorwhen we explicitly check the type, but a plainTypeErrorwhen we haven't explicitly checked, for example.So, in short:
I think it makes sense to extend the built-in exception hierarchy with more specific ones, but not to duplicate what's already there.
I don't see a huge value in the common
AnsysErrorbase class, and would rather each exception type inherit from its most suitable built-in exception type (e.g.OSErrorfor system-level errors, ...)It may make sense to pull this discussion into a separate issue - for this PR I think it's okay to keep the
ProductInstanceError.