-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[MAINTENANCE] Add suite_name
to ExpectationSuiteValidationResult
#9677
Changes from 5 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -503,13 +503,15 @@ | |
evaluation_parameters: Dict of Evaluation Parameters used to produce these results, or None. | ||
statistics: Dict of values describing the results. | ||
meta: Instance of ExpectationSuiteValidationResult, a Dict of meta values, or None. | ||
|
||
batch_id: A unique identifier for the batch of data that was validated. | ||
result_url: A URL where the results are stored. | ||
""" # noqa: E501 | ||
|
||
def __init__( # noqa: PLR0913 | ||
self, | ||
success: Optional[bool] = None, | ||
results: Optional[list] = None, | ||
success: bool, | ||
results: list[ExpectationValidationResult], | ||
suite_name: str, | ||
evaluation_parameters: Optional[dict] = None, | ||
statistics: Optional[dict] = None, | ||
meta: Optional[ExpectationSuiteValidationResult | dict] = None, | ||
|
@@ -518,21 +520,16 @@ | |
id: Optional[str] = None, | ||
) -> None: | ||
self.success = success | ||
if results is None: | ||
results = [] | ||
self.results = results | ||
if evaluation_parameters is None: | ||
evaluation_parameters = {} | ||
self.evaluation_parameters = evaluation_parameters | ||
if statistics is None: | ||
statistics = {} | ||
self.statistics = statistics | ||
if meta is None: | ||
meta = {} | ||
self.suite_name = suite_name | ||
self.evaluation_parameters = evaluation_parameters or {} | ||
self.statistics = statistics or {} | ||
meta = meta or {} | ||
ensure_json_serializable(meta) # We require meta information to be serializable. | ||
self.meta = meta | ||
self.batch_id = batch_id | ||
self.result_url = result_url | ||
self.id = id | ||
self._metrics: dict = {} | ||
|
||
def __eq__(self, other): | ||
|
@@ -613,7 +610,7 @@ | |
) -> ExpectationSuiteValidationResult: | ||
validation_results = [result for result in self.results if not result.success] | ||
|
||
successful_expectations = sum(exp.success for exp in validation_results) | ||
successful_expectations = sum(exp.success or False for exp in validation_results) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this required? do some ValidationResults have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah we'd need a separate effort to clean up the child |
||
evaluated_expectations = len(validation_results) | ||
unsuccessful_expectations = evaluated_expectations - successful_expectations | ||
success = successful_expectations == evaluated_expectations | ||
|
@@ -632,6 +629,7 @@ | |
return ExpectationSuiteValidationResult( | ||
success=success, | ||
results=validation_results, | ||
suite_name=self.suite_name, | ||
evaluation_parameters=self.evaluation_parameters, | ||
statistics=statistics, | ||
meta=self.meta, | ||
|
@@ -654,11 +652,11 @@ | |
class ExpectationSuiteValidationResultSchema(Schema): | ||
success = fields.Bool() | ||
results = fields.List(fields.Nested(ExpectationValidationResultSchema)) | ||
suite_name = fields.String(required=True, allow_none=False) | ||
evaluation_parameters = fields.Dict() | ||
statistics = fields.Dict() | ||
meta = fields.Dict(allow_none=True) | ||
id = fields.UUID(required=False, allow_none=True) | ||
checkpoint_name = fields.String(required=False, allow_none=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was wrong - it might be in meta but shouldn't be at the top level |
||
|
||
# noinspection PyUnusedLocal | ||
@pre_dump | ||
|
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.
These should be guaranteed fields - what is a result if we don't have results, an origin suite, and a success metric?