-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix: expression type in system v2 #162
Conversation
@@ -204,7 +205,8 @@ def to_dto( | |||
else: | |||
rates = [ | |||
Expression.multiply( | |||
Expression.setup_from_expression(self.rate), Expression.setup_from_expression(rate_fraction) | |||
Expression.setup_from_expression(operational_setting.total_system_rate), |
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.
oops, forgot to mention I fixed a bug
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.
nice to catch this..another "pros" of having a type/alias rather than typing the Union etc all the time...
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.
Ideally separate commit though...
@@ -21,7 +22,7 @@ def to_dto(self, **kwargs): | |||
|
|||
|
|||
class OperationalConditionBase(YamlBase): | |||
condition: Optional[str] = Field( |
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.
:O
3c7cb50
to
89de208
Compare
89de208
to
1de9df3
Compare
Why is this pull request needed?
Only str was allowed for expressions in the yaml models for system v2 (yellow mark because of json-schema)
What does this pull request change?
Sets the type of expression to a union of int, float and str
Issues related to this change: