Exception improvements
- Dominant language
- Python
- Stars
- 497
- Forks
- 77
- Avg merge
- 22h 37m
- Merged PRs (30d)
- 45
Description
A few things I'm not a huge fan of currently with errors
1. There doesn't seem to be a class hierarchy for exceptions raised. Most libraries tend to have a "`PyFluentError`" that all custom exceptions inherit from in case you want to catch all the errors from the library.
2. Auditing [the code](https://github.com/search?q=repo%3Aansys%2Fpyfluent+%2Fclass+%5Cw%2B%5C%28%5Cw%2BError%2F&type=code) there seem to be a few exceptions that are entirely redundant and can be replaced with their std lib counter parts. e.g.
In this case this should just be a `ValueError`. Same for `UnexpectedKeywordArgument` that should just be a `TypeError`
3. There are also a lot of custom `TypeErrors` which isn't something you typically see. I would air on the side of just raising the exception directly for 2 reasons, it's shorter to read along with avoiding pollution the namespace with a bunch of errors which hurts IDE users who have to look through more options to find what they are looking for. I don't think anyone in normal code is catching a `WaitTypeError` so it's not losing functionality.
> Some developers may argue that, in this example, you can use the built-in `ValueError` instead of defining your own custom exception, and they might be right. In general, you’ll define custom exceptions when you want to point out project-specific errors or exceptional situations. - https://realpython.com/python-raise-exception/#choosing-the-exception-to-raise-built-in-vs-custom
I don't think the cases in this library really justify another error here
If you want to keep the consistent error messages maybe have all the error messages as strings in exceptions.py and just import those in the file that raises them and then in tests if that's something that needs doing.
I am heavily in favour of getting rid of most of these all together as they should be caught by type checkers (though unsure how this works with pyconsole)
4. The `InvalidSolutionVariableNameError` error could be slightly confusing due to its suffix, maybe there's a better name, `SolutionVariableNotFoundError`?
5. Conditional usage of `Exception.add_note()` will help to improve the already good current situation with guiding users towards the correct attribute name using the mechanism for enriching exceptions on versions greater than or equal to 3.11
Contributor guide
Research direction
Start by auditing the custom exceptions referenced in exceptions.py and the linked code search, then review the existing tests for their usage. Define the exception hierarchy, decide which custom errors can become built-ins, assess the proposed rename, and check conditional Exception.add_note() support for Python 3.11+. Done means the exception API and related tests consistently reflect the agreed decisions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100