-
Notifications
You must be signed in to change notification settings - Fork 62
feat: Add CompoundMeasureValueFilter #1304
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
Conversation
Adding CompoundMeasureValueFilter definition, which can be used to define several numerical conditions in a single filter. JIRA: LX-2073 risk: low
eadaa4b to
642305a
Compare
| return afm_models.MeasureValueCondition(range=range_body, _check_type=False) | ||
|
|
||
| def description(self) -> str: | ||
| not_between = "not" if self.operator == "NOT_BETWEEN" else "" |
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.
Based on usage in the example below, maybe "not " (missing space)?
|
|
||
| @attrs.define(frozen=True, slots=True) | ||
| class MetricValueComparisonCondition: | ||
| operator: 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.
Consider using StrEnum or Literal. I suspect that the values will not change.
| if "compoundMeasureValueFilter" in filter_dict: | ||
| f = filter_dict["compoundMeasureValueFilter"] |
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.
Simplification:
| if "compoundMeasureValueFilter" in filter_dict: | |
| f = filter_dict["compoundMeasureValueFilter"] | |
| f = filter_dict.get("compoundMeasureValueFilter" , {}) |
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.
Not sure if I get this one, this would change the behaviour or not?
For non-compound filters, f becomes {} and the code would then hit f["measure"] (or similar) and fail with a KeyError, instead of cleanly falling through to the other filter types.
It also hides bugs: defaulting to {} makes “missing key” look like “present but empty”.
Adding CompoundMeasureValueFilter definition, which can be used to define several numerical conditions in a single filter.
JIRA: LX-2073
risk: low