-
Notifications
You must be signed in to change notification settings - Fork 4
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
[NEAT-27] DMS sheet validation #273
Conversation
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.
Couple of comments regarding exception handling.
] | ||
|
||
|
||
StrListType = Annotated[ | ||
list[str], | ||
BeforeValidator(lambda value: [entry.strip() for entry in value.split(",")] if isinstance(value, str) else value), | ||
PlainSerializer( |
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.
Is this used when exporting rules to e.g. Excel?
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.
It is used when you call .model_dump
|
||
value_types = {prop.value_type for prop in properties if prop.value_type} | ||
if len(value_types) > 1: | ||
exceptions.append( |
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.
Define these as NeatExceptions
) | ||
list_definitions = {prop.is_list for prop in properties if prop.is_list is not None} | ||
if len(list_definitions) > 1: | ||
exceptions.append( |
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.
Same here
) | ||
nullable_definitions = {prop.nullable for prop in properties if prop.nullable is not None} | ||
if len(nullable_definitions) > 1: | ||
exceptions.append( |
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.
same here
) | ||
default_definitions = {prop.default for prop in properties if prop.default is not None} | ||
if len(default_definitions) > 1: | ||
exceptions.append( |
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.
same here
) | ||
index_definitions = {",".join(prop.index) for prop in properties if prop.index is not None} | ||
if len(index_definitions) > 1: | ||
exceptions.append( |
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.
same
) | ||
constraint_definitions = {",".join(prop.constraint) for prop in properties if prop.constraint is not None} | ||
if len(constraint_definitions) > 1: | ||
exceptions.append( |
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.
same as aboce
|
||
if exceptions: | ||
exception_str = "\n".join(exceptions) | ||
raise ValueError(f"Inconsistent container(s): {exception_str}") |
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.
You can actually use ExceptionGroup
for this,
For python lower than 3.11, make sure to import this:
if sys.version_info < (3, 11):
from exceptiongroup import ExceptionGroup
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.
I tried, but pydantic was not happy.
# Conflicts: # cognite/neat/rules/models/_rules/dms_architect_rules.py # tests/tests_unit/rules/test_models/test_dms_architect_rules.py
Validations added:
In addition, allow you to only define index and constraint on a container property only the first row you define it. You can set the rest to zero.
Instead of
You can now do: