FEATURE: Added profile build for "has_no_outliers" check#1317
Conversation
mwojtyczka
left a comment
mwojtyczka
left a comment
There was a problem hiding this comment.
Re-review of the has_no_outliers profile builder. The MAD extraction and option-constant refactor are clean, but there are two blocking issues plus some cleanup. Most points are inline; two that can't be anchored to this diff:
🔴 Blocking — has_no_outliers profiles never become checks
DQGenerator._checks_mapping (src/databricks/labs/dqx/profiler/generator.py) maps only is_not_null, is_in, min_max, is_not_null_or_empty, is_not_empty, is_unique, and generate_dq_rules() skips any profile whose name isn't a key. This PR builds a DQProfile(name="has_no_outliers") but never adds a mapping entry / dq_generate_has_no_outliers, and doesn't touch generator.py — so the profile is silently dropped and no check is generated end-to-end, which is the PR's stated goal.
Test coverage
Unit tests cover only the non-numeric early-exit; the positive / threshold-negative paths and the MAD math are integration-only, and nothing tests the profile → generate_dq_rules → check path. That's exactly why the mapping gap above and the filter crash (inline) both slip through — the tests call the builder directly with a single-column frame and no cross-column filter. A generate-path test and a cross-column-filter test would catch both blocking issues.
See inline comments for the filter crash (blocking), numeric-type narrowing, ratio denominator, the Any convention, and module placement.
mwojtyczka
added
under-review
ghanse
left a comment
ghanse
left a comment
There was a problem hiding this comment.
This will be a nice addition. Agree with the other feedback. Left a few small comments.
…qx into feature/has_no_outliers
|
@mwojtyczka @mwojtyczka Thank you so much for your review. May I ask for second round? |
mwojtyczka
left a comment
mwojtyczka
left a comment
There was a problem hiding this comment.
Re-review of 794cf15. All prior findings (mine + @ghanse's) are addressed — the profiler-filter crash is fixed, the builder uses NumericType, the outlier ratio uses count_non_null consistently, dict[str, Any] is gone, calculations_utils.py was folded into profiling_utils.py (MAD shared by the check and the builder), and the requested tests (numeric-type parametrization, all-nulls→None) were added. Thanks for the thorough turnaround.
New findings below. #1 (filter dropped) and #2 (outliers_ratio not plumbed through ProfilerConfig/ProfilerRunner) are the two I'd fix before merge — they make the feature behave inconsistently with the rest of the profiler and silently ignore a documented config. The rest are a perf/toggle decision, two edge cases (zero-MAD, Decimal bounds), and doc/style cleanup.
Minor (not inline): the 0.01 default is duplicated between DEFAULT_PROFILE_OPTIONS and the inline .get(..., 0.01) fallback; dq_generate_has_no_outliers has an unused params (params = params or {} is dead); and column_name is interpolated into the logged DQProfile.description without newline sanitization (CWE-117, low — schema-derived).
|
@mwojtyczka Im sorry for failed integration tests, I'll work on fix |
no worries, let me know if you need help |
The has_no_outliers profiler feature is opt-in (PROFILE_OPTION_HAS_NO_OUTLIERS defaults to False), but the integration tests were written assuming it was enabled by default. They expected has_no_outliers entries in the profile output (or asserted exactly one profile) without setting the flag, so the builder short-circuited and emitted nothing -> DQProfile list mismatches and assert 0 == 1. Pass "has_no_outliers": True in the options of every profiler test that expects the profile, so they exercise the feature explicitly. Also enable it in the negative threshold test so it actually verifies suppression instead of passing coincidentally while the feature is disabled. Co-authored-by: Isaac
I think you just missed enabling it in the test, just updated |
3 participants
Changes
Summary of changes:
has_no_outliersprofile build to generate corresponding check;Linked issues
Resolves #977
Tests
Documentation and Demos