Feature: Categorical values distribution - #1450
Conversation
mwojtyczka
left a comment
mwojtyczka
left a comment
There was a problem hiding this comment.
Automated code review (high-effort recall pass) for is_in_distribution. 6 findings inline, ranked most-severe first. Correctness items #1–#3 are marked plausible — verified by reading the sibling dataset-check closures rather than executing Spark locally.
This review was generated with assistance from Claude Code.
mwojtyczka
added
the
under-review
IvannKurchenko
changed the title
|
Hello, @mwojtyczka! Thanks for a review. The previous feedback has been addressed. Would it be possible to have another round? |
mwojtyczka
left a comment
mwojtyczka
left a comment
There was a problem hiding this comment.
Follow-up review pass
Resolved (verified against HEAD — threads now closed): the earlier .collect()/streaming (L599), driver-baked verdict (L646), df.schema[expr] KeyError (L565), implicit int upcast (L640), and missing -> None (L691) concerns are all addressed by the refactor to lazy conditional aggregation + explicit key casts. Marked those five threads resolved.
Still open: the YAML fixture thread (all_dataset_checks.yaml:267) is only partially addressed — the new second entry (TVD=0.5, distance=1.0) guards against a comparison inversion, but no fixture entry actually flags a mismatch end-to-end (both entries use distance: 1.0 and pass by design). Left open so you can decide whether positive-detection belongs here or in the integration test.
New findings (inline): two correctness edge cases (0.0-probability key under impute=False; is_close abs_tol=0 at distance=0) plus three minor items.
mwojtyczka
left a comment
mwojtyczka
left a comment
There was a problem hiding this comment.
Left some additional comments, previous comments resolved except one only partilally addressed.
|
@mwojtyczka May I ask for another review round, please? Your previous comments has been addressed. |
mwojtyczka
added
the
Approved to Merge
…DX build
The '{A: 0.5, B: 0.3}' example in the is_in_distribution docstring was
emitted verbatim into docs/reference/api/check_funcs.md, where MDX parsed
'{...}' as a JS expression and failed ('Could not parse expression with
acorn'), breaking the Build Docusaurus CI job. Wrap the literal in inline
code so MDX treats it as text, matching the existing pattern in
has_valid_json_schema. Verified with a full 'yarn build' (docs build now
succeeds).
Co-authored-by: Isaac <no-reply@databricks.com>
2 participants
Changes
New check: Categorical values distribution check at dataset level using TDV measurement.
Linked issues
Resolves #1344
Tests
Documentation and Demos