Skip to content
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

Merged
merged 2 commits into from
Sep 12, 2023
Merged

fix: expression type in system v2 #162

merged 2 commits into from
Sep 12, 2023

Conversation

jsolaas
Copy link
Contributor

@jsolaas jsolaas commented Sep 11, 2023

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:

@jsolaas jsolaas requested a review from a team as a code owner September 11, 2023 17:47
@@ -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),
Copy link
Contributor Author

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

Copy link
Collaborator

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...

Copy link
Collaborator

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(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:O

@jsolaas jsolaas force-pushed the fix/expression-json-schema branch from 3c7cb50 to 89de208 Compare September 12, 2023 09:24
@jsolaas jsolaas force-pushed the fix/expression-json-schema branch from 89de208 to 1de9df3 Compare September 12, 2023 09:28
@jsolaas jsolaas merged commit 56da4b2 into main Sep 12, 2023
@jsolaas jsolaas deleted the fix/expression-json-schema branch September 12, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants