fix(context): compare allowed values by type for numeric and boolean columns - #380
Merged
Merged
Conversation
…columns
_check_allowed_values stringified both sides, so a float column's 1.0
('1.0') never matched a declared '1' and a bool column's True ('True')
never matched 'true'. Every value in those columns was reported as a
violation, and the stringified value_set was passed through to the
Great Expectations and dbt exporters, which then failed on every row too.
Now the comparison follows the column dtype:
- bool / nullable boolean columns: allowed entries map case-insensitively
from true/false/yes/no/t/f/y/n/1/0 to bools;
- other numeric columns: allowed entries are parsed as finite numbers and
compared with exact numeric equality (1 matches 1.0);
- all other columns keep the existing string comparison.
Missing values are still ignored. extra["value_set"] and observed_value
now carry the typed values, so exported GX expectations and dbt
accepted_values tests use numbers and booleans. Allowed entries that do
not convert stay in value_set as strings but cannot match typed values.
Closes #255
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
FreshData benchmark report —
|
| fixture | n_rows | n_cols | p50 s | p95 s | peak MB | repair % | false-repair % | preserve % | trust | monotonic | export % |
|---|
Authored-code reduction (Metric 6)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fd.validate'sallowed_valuescheck stringified both the column and the declared values, so float columns (1.0vs1) and bool columns (Truevstrue) reported every row as a violation. The stringifiedvalue_setalso leaked intoexport_gx_suite(andexport_dbt_tests), producing expectations that fail on every row.The comparison now follows the column dtype:
boolean: allowed entries map case-insensitively fromtrue/false/yes/no/t/f/y/n/1/0.1matches1.0).Missing values are still ignored.
extra["value_set"]andobserved_valuenow carry typed values, so GXexpect_column_values_to_be_in_setgets[1, 2, 3]/[true, false]and dbtaccepted_valuesemits unquoted numbers/booleans. Allowed entries that don't convert stay invalue_setas strings but can't match typed values.Tests
New
tests/context/test_allowed_values_typed.py: issue repro, float column with1.5, 2.5, int column (typedvalue_set/observed_value), nullableInt64with missing values, non-numeric entry on a numeric column, bool spellings (true/false,yes/no, mixed case,1/0,Y/n), nullableboolean, string column unchanged, GX export typedvalue_set, dbt export typed scalars.Verification
ruff check .— passmypy src/freshdata— no issuespytest -m "not online and not large"— py3.12: 4261 passed, 6 skipped; py3.9: 4257 passed, 10 skipped (before rebasing onto current main; context and integration suites re-run after the rebase on both)Closes #255