-
Notifications
You must be signed in to change notification settings - Fork 1
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
Separate out entity models in SOFT and DLite #74
Conversation
Create a "minimum set" of fields for the versioned SOFT models and then create the DLite implementations of these separately.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #74 +/- ##
==========================================
- Coverage 91.02% 90.78% -0.25%
==========================================
Files 17 21 +4
Lines 981 955 -26
==========================================
- Hits 893 867 -26
Misses 88 88
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
||
@model_validator(mode="before") | ||
@classmethod | ||
def _check_cross_dependent_fields(cls, data: Any) -> Any: |
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.
_cross_check_dependent_fields seems especially long for something that is relatively simple. Cross check also implies something more complex (e.g that the name and namespace are consistent with the version according to a complex rule or something)
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 should be noted that the content and logic of the validators have not been changed. They have merely been moved around a bit to where they not belong. However, it may still be fine to address in this PR, for sure :)
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.
Essentially, namespace+version+name
in that specific order MUST be equal to uri
if all of these fields are given. That is what's being checked :)
shape: Annotated[ | ||
list[str] | None, | ||
Field( | ||
description=( |
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.
This description seems really complex. Maybe something like "Defines the dimensions of a multi-dimensional property using a list of expressions. "
"Each expression in the list corresponds to a dimension and can be a named dimension "
"(e.g., 'H', 'K', 'L') or an arithmetic expression involving these dimensions. "
"For example, if a property's shape is defined as ['K', 'H+1'] and the dimensions of the entity "
"are assigned values 'H=2', 'K=2', 'L=6', the resulting shape of the property will be [2, 3]. "
"This allows for dynamic sizing of properties based on the entity's dimensions. <ChatGPT said this, is it true?>" I'm personally a bit confused by how multi-dimensions work now
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.
Technically this comment is not in the scope of the PR since it was already there but I point it out now...
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.
Feel free to open an issue on this :)
Closes #64
Create a "minimum set" of fields for the versioned SOFT models and then create the DLite implementations of these separately.
This should be implemented in conjunction with #70