feat(ACI): Handle anomaly detection data condition in validator #93886
Conversation
| if isinstance(self.initial_data, list) and self.initial_data: | ||
| return self.initial_data[0].get("type") |
There was a problem hiding this comment.
self.initial_data is a list of conditions in some cases (e.g. the case where we're subclassing it for the MetricIssueComparisonConditionValidator)
There was a problem hiding this comment.
Might be a silly question, but how do we know to get the type using for index 0? I'm assuming we know there's only one condition in the list?
There was a problem hiding this comment.
This is a bit of an assumption yeah - for anomaly detection we may have 2 data conditions but their type would both be 'anomaly detection'. I don't think we'd ever have a case where we have 1 anomaly detection data condition and 1 of another type, right?
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #93886 +/- ##
===========================================
+ Coverage 76.56% 88.03% +11.46%
===========================================
Files 10338 10333 -5
Lines 597261 598430 +1169
Branches 23193 23193
===========================================
+ Hits 457316 526844 +69528
+ Misses 139484 71125 -68359
Partials 461 461 |
| if isinstance(self.initial_data, list) and self.initial_data: | ||
| return self.initial_data[0].get("type") |
There was a problem hiding this comment.
Might be a silly question, but how do we know to get the type using for index 0? I'm assuming we know there's only one condition in the list?
| AbstractDataConditionValidator[float, DetectorPriorityLevel] | ||
| ): | ||
| supported_conditions = frozenset((Condition.GREATER, Condition.LESS)) | ||
| class MetricIssueComparisonConditionValidator(BaseDataConditionValidator): |
There was a problem hiding this comment.
Why the switch to BaseDataConditionValidator here?
There was a problem hiding this comment.
I did this because I need to use the base class's validate_comparison method (line 49) for dict input
| "condition_result": 25, | ||
| "condition_group_id": self.data_condition_group.id, | ||
| **self.valid_data, | ||
| "conditionResult": 25, |
There was a problem hiding this comment.
Why did this change from snake case to camel case?
There was a problem hiding this comment.
good question, it's a mix of both throughout the payloads of many tests 🤔 updated to just be all camel
Update the metric issue validator to handle anomaly detection data condition input - the main difference with these is that
comparisonis a dictionary rather than a number.