-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use exceptions for UnitAlgebra errors #1043
Use exceptions for UnitAlgebra errors #1043
Conversation
CLANG-FORMAT TEST - PASSED |
CMAKE-FORMAT TEST - PASSED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. This will be a nice addition.
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MR-2
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MT-2
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-macro_withsstcore
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core_Make-Dist
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_Clang-Format_sst-core
Build InformationTest Name: SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-elements
Build InformationTest Name: SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-macro_withsstcore
Using Repos:
Pull Request Author: berquist |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MR-2
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MT-2
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-macro_withsstcore
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core_Make-Dist
Build InformationTest Name: SST__AutotestGen2_NewFW_sst-test_Clang-Format_sst-core
Build InformationTest Name: SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-elements
Build InformationTest Name: SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-macro_withsstcore
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ feldergast ]! |
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - Forked Repos cannot Automerge |
Closes #1037
Here is a proposal for adding exceptions to the core public API. Doxygen is missing until we decide on the form of exceptions.
Thoughts:
UnitAlgebra
, I've added them inside theUnitAlgebra
class rather than just the header.UnitAlgebraException
in case a consumer (elements, some other code) wants to catch a more generic exception type rather than one of the more specific types.UnitAlgebraException
inherits fromlogic_error
rather thanexception
since these are fundamentally not runtime errors.PyErr_SetString
should always beexception_instance.what()
.For the tests, I used
assert
statements rather than the existingcorrect_throw
idiom.