Refactors the controls module, adding custom errors #17
Conversation
There was a problem hiding this comment.
Looks good to me. There is a library called termcolor which I have used to print coloured text on to the terminal. It may offer more formatting options if you are interested. But it works on exactly the same principle of using ANSI color codes under the hood.
RAT/controls.py
Outdated
| model = controls[procedure](**properties) | ||
| except KeyError: | ||
| members = list(Procedures.__members__.values()) | ||
| allowed_values = ', '.join([repr(member.value) for member in members[:-1]]) + f' or {members[-1].value!r}' |
There was a problem hiding this comment.
str cat is not needed below should work
f"{', '.join([repr(member.value) for member in members[:-1]])} or {members[-1].value!r}"
RAT/project.py
Outdated
| error_string = formatted_pydantic_error(e) | ||
| # Use ANSI escape sequences to print error text in red | ||
| print('\033[31m' + error_string + '\033[0m') | ||
| traceback_string = formatted_traceback() |
There was a problem hiding this comment.
I think we still need to propagate the exception with the new error message, otherwise if logging is switched off or an app has no console (typical GUI app), there will be no way for upstream packages to know there is a problem. I am not sure if there is a specific reason the exception is not re-raised
maybe
tb = exc.__traceback__
raise ValueError(custom_msg).with_traceback(tb) from None
or
error_string = formatted_pydantic_error(exc, custom_msgs)
raise ValueError(error_string) from exc
PS: I used ValueError cause I could not create ValidationError. This way I think the logging is not need
There was a problem hiding this comment.
Pydantic recommends this way of handling ValidationErrors: https://docs.pydantic.dev/latest/errors/errors/ I'm inclined to agree that raising the error is a better cause of action in general, but I don't know how to do it with the error message we want - I'll have a look and see what I can find though.
There was a problem hiding this comment.
i.e. - it's dead easy to raise the exception with the standard error message, but that it what we want to change.
There was a problem hiding this comment.
I don't mind if the exception is propagated as is and the upstream modifies the message but their suggestions are for end users only and will not help if there is no console which will be the case with RASCAL. If the best solution is to reraise the exception without modification then you have to modify this code so it doesn't print the same thing twice
There was a problem hiding this comment.
You could also make a custom RAT validation error and copy the traceback
…rror messages when raising a ValidationError
DrPaulSharp
changed the title
No description provided.