-
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
[DOCS] puts 1.0 code under test for how to define Expectations #10229
[DOCS] puts 1.0 code under test for how to define Expectations #10229
Conversation
…efault` Data Source" into a script under test.
… test; adds examples around testing Expectations that use Expectation Parameters.
…Suites into a script under test.
✅ Deploy Preview for niobium-lead-7998 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #10229 +/- ##
===========================================
- Coverage 79.67% 79.65% -0.02%
===========================================
Files 454 454
Lines 39507 39507
===========================================
- Hits 31476 31469 -7
- Misses 8031 8038 +7 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…ons that utilize an Expectation Parameters dictionary.
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.
Left a couple comments. Nothing blocking, but figured I'd verify if the one part is commented out before approving
# <snippet name="docs/docusaurus/docs/core/define_expectations/_examples/create_an_expectation.py - dynamic expectations"> | ||
passenger_expectation = gx.expectations.ExpectColumnMaxToBeBetween( | ||
column="passenger_count", | ||
min_value={"$PARAMETER": "expect_passenger_max_to_be_above"}, |
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.
Just floating an idea: Does this become easier if you make a constant for expect_passenger_max_to_be_above
, etc, to show how they are linked to what is passed in below?
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.
As in:
EXPECT_PASSENGER_MAX_TO_BE_ABOVE="expect_passenger_max_to_be_above"
...
passenger_expectation = gx.expectations.ExpectColumnMaxToBeBetween(
column="passenger_count",
min_value={"$PARAMETER": EXPECT_PASSENGER_MAX_TO_BE_ABOVE},
...
expectation_parameters = {
EXPECT_PASSENGER_MAX_TO_BE_ABOVE: 1
}
?
I'm not certain if that's easier to comprehend or not, but I'm not against making the change if you think it is.
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.
Maybe something like MAX_PASSENGER_PARAMETER
, but that's the idea. I don't have a strong opinion here, I guess. If this were production code, I'd do it, but maybe that doesn't translate perfectly to documentation. Totally your call.
) | ||
fare_expectation = gx.expectations.ExpectColumnMaxToBeBetween( | ||
column="fare", | ||
min_value={"$PARAMETER": "expect_fare_max_to_be_above"}, |
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.
Looking at this again, I'm wondering if we should consider abstracting away the $PARAMETER
part of this before launch? Like what if it just became min_value=gx.expectations.parameter("expect_fare_max_to_be_above"),
? Not blocking for this PR at all.
…ctations # Conflicts: # docs/docusaurus/docs/components/examples_under_test.py # docs/docusaurus/docs/core/define_expectations/_retrieve_a_batch_of_test_data/_from_a_batch_definition.py
Description
pandas_default
Data SourceDefinition of done:
invoke lint
(usesruff format
+ruff check
)