[BUGFIX] Exclude nulls and fix the rank offset in the SQLite quantile metric - #12008
Conversation
… metric The SQLite branch of column.quantile_values computed offsets as quantile * table_row_count - 1, which is a float that SQLite truncates, so the effective index was floor(q*n)-1 where percentile_disc is ceil(q*n)-1. Every quantile whose q*n was not a whole number came back one rank low. The query also had no null filter, and table_row_count counts nulls. SQLite sorts nulls first, so on a column containing nulls the offsets pointed into the null prefix and the metric returned None, which surfaced as a TypeError when the result was range-checked. Rank over the non-null values instead. The count comes from a column_values.nonnull.count dependency, declared the way column_median.py already declares it and requested only for SQLite, since no other dialect reads it. The rank is computed with fractions.Fraction rather than in floating point, because 0.56 * 25 is 14.000000000000002 and a plain math.ceil would skip a rank. Apply the same null filter to the MySQL percent_rank CTE, which had the same omission. On null-free data it is a no-op.
👷 Deploy request for niobium-lead-7998 pending review.Visit the deploys page to approve it
|
github-actions
Bot
added
the
cla-not-signed
|
All committers have signed the CLA. ✅ |
SreeramaYeshwanthGowd
changed the title
|
@cla-bot check |
github-actions
Bot
added
cla-signed
and removed
cla-not-signed
Reading the non-null count with metrics.get meant a missing dependency was indistinguishable from an empty column, and the SQLite branch would return NaN for every quantile instead of failing. Subscript instead, so the dependency graph has to supply it. Add a test for an all-null column, which previously raised a TypeError from _validate on SQLite and now reports null observed values with success false, matching the pandas backend. Rename the rank test. Pandas uses nearest interpolation and does not select the first rank reaching the quantile in general, it only agrees with percentile_disc at the quantiles that test uses, so the old name claimed more than it checked.
|
Adding the detail I left out of the description, since this changes observed values on a supported backend and that should be on the record before review. Reproduction on developimport pandas as pd, sqlalchemy as sa
import great_expectations as gx
import great_expectations.expectations as gxe
pd.DataFrame({"amount": range(1, 16)}).to_sql(
"tbl", sa.create_engine("sqlite:////tmp/q.db"), index=False
)
ctx = gx.get_context(mode="ephemeral")
ds = ctx.data_sources.add_sqlite("s", connection_string="sqlite:////tmp/q.db")
batch = ds.add_table_asset("a", table_name="tbl").add_batch_definition_whole_table("b").get_batch()
batch.validate(gxe.ExpectColumnQuantileValuesToBeBetween(
column="amount",
quantile_ranges={"quantiles": [0.25, 0.5, 0.75], "value_ranges": [[0, 99]] * 3},
)).to_json_dict()["result"]["observed_value"]develop returns Put nulls in the same column and develop returns Behaviour changeThis is not only a null fix. Null free SQLite columns also change wherever An all null column also changes. It previously raised the I checked the one in tree consumer of the metric. CostSQLite now resolves MySQLThe CIThere is no signal on the change itself yet. |
|
thanks for the PR @SreeramaYeshwanthGowd. I ran CI, and it looks like static analysis is failing. I'll give it a thorough review once CI is passing. |
to_json_dict() returns a union type that mypy cannot narrow through two chained string indices, which is why every other assertion in this module compares the whole "result" dict instead. Match that pattern here too.
|
@joshua-stauffer. Thanks for triggering it. I've fixed it with new test assertion. Ready for another run. |
2 participants
ExpectColumnQuantileValuesToBeBetweenis one rank low on SQLite and raises on columns with nulls.Root cause
column_quantile_values.py:332offsets byquantile * table_row_count - 1. That truncates tofloor(q*n)-1wherepercentile_discgivesceil(q*n)-1, and nulls are not filtered.column_median.py:60-70does this null-safely already.Fix
Ranks now use
column_values.nonnull.countand exactFractionarithmetic. MySQL's CTE gets the same filter.Tests
Three integration tests, two of which fail on develop.
invoke lint(usesruff format+ruff check)tests/integration/data_sources_and_expectations