-
Notifications
You must be signed in to change notification settings - Fork 5
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
Refactors the controls module, adding custom errors #17
Changes from 5 commits
1e24ab3
b78a6ae
bcd549c
1dbe8dc
d25150a
bcacf29
ffcf677
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
from RAT.classlist import ClassList | ||
from RAT.controls import Controls | ||
from RAT.project import Project | ||
import RAT.controls | ||
import RAT.models |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,14 +3,15 @@ | |
import collections | ||
import copy | ||
import functools | ||
import logging | ||
import numpy as np | ||
import os | ||
from pydantic import BaseModel, ValidationInfo, field_validator, model_validator, ValidationError | ||
from typing import Any, Callable | ||
|
||
from RAT.classlist import ClassList | ||
import RAT.models | ||
from RAT.utils.custom_errors import formatted_pydantic_error | ||
from RAT.utils.custom_errors import formatted_pydantic_error, formatted_traceback | ||
|
||
try: | ||
from enum import StrEnum | ||
|
@@ -524,11 +525,12 @@ def wrapped_func(*args, **kwargs): | |
try: | ||
return_value = func(*args, **kwargs) | ||
Project.model_validate(self) | ||
except ValidationError as e: | ||
except ValidationError as exc: | ||
setattr(class_list, 'data', previous_state) | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
or
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could also make a custom RAT validation error and copy the traceback |
||
error_string = formatted_pydantic_error(exc) | ||
logger = logging.getLogger(__name__) | ||
logger.error(traceback_string + error_string + '\n') | ||
except (TypeError, ValueError): | ||
setattr(class_list, 'data', previous_state) | ||
raise | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,44 @@ | ||
"""Defines routines for custom error handling in RAT.""" | ||
|
||
from pydantic import ValidationError | ||
import traceback | ||
|
||
|
||
def formatted_pydantic_error(error: ValidationError) -> str: | ||
def formatted_pydantic_error(error: ValidationError, custom_error_messages: dict[str, str] = None) -> str: | ||
"""Write a custom string format for pydantic validation errors. | ||
|
||
Parameters | ||
---------- | ||
error : pydantic.ValidationError | ||
A ValidationError produced by a pydantic model | ||
A ValidationError produced by a pydantic model. | ||
custom_error_messages: dict[str, str], optional | ||
A dict of custom error messages for given error types. | ||
|
||
Returns | ||
------- | ||
error_str : str | ||
A string giving details of the ValidationError in a custom format. | ||
""" | ||
if custom_error_messages is None: | ||
custom_error_messages = {} | ||
num_errors = error.error_count() | ||
error_str = f'{num_errors} validation error{"s"[:num_errors!=1]} for {error.title}' | ||
|
||
for this_error in error.errors(): | ||
error_type = this_error['type'] | ||
error_msg = custom_error_messages[error_type] if error_type in custom_error_messages else this_error["msg"] | ||
|
||
error_str += '\n' | ||
if this_error['loc']: | ||
error_str += ' '.join(this_error['loc']) + '\n' | ||
error_str += ' ' + this_error['msg'] | ||
error_str += f' {error_msg}' | ||
|
||
return error_str | ||
|
||
|
||
def formatted_traceback() -> str: | ||
"""Takes the traceback obtained from "traceback.format_exc()" and removes the exception message for pydantic | ||
ValidationErrors. | ||
""" | ||
traceback_string = traceback.format_exc() | ||
return traceback_string.split('pydantic_core._pydantic_core.ValidationError:')[0] |
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.
str cat is not needed below should work
f"{', '.join([repr(member.value) for member in members[:-1]])} or {members[-1].value!r}"