fix: make binningUdf/maxBins optional in hasNumberOfDistinctValues/hasHistogramValues (#81) - #274
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Ready for review. Makes |
|
This PR has been inactive for 60 days. It will be closed in 14 days if there is no further activity. If you are still working on this, please push an update or comment to keep it open. |
hasNumberOfDistinctValues and hasHistogramValues forced callers to pass binningUdf and maxBins, although Deequ's Scala API marks both as optional with defaults (binningUdf=None, maxBins=Histogram.MaximumAllowedDetailBins). Default to the Scala-side defaults via the apply$default$N accessors when the args are omitted, mirroring the existing satisfies() pattern. Existing positional callers remain backward compatible. Fixes awslabs#81
f5d9258 to
54d2d1b
Compare
|
rebased onto the latest master — green and mergeable. makes binningUdf/maxBins optional (matching deequ's scala defaults), with tests. still relevant and i'm happy to keep it current; would appreciate a review when there's bandwidth. |
1 participant
Problem
hasNumberOfDistinctValuesandhasHistogramValuesforce the caller to passbinningUdfandmaxBins, but Deequ's Scala API defines both with defaults. So a natural call:raises
TypeError: ... missing 2 required positional arguments: 'binningUdf' and 'maxBins'. Reported in #81.Fix
Make
binningUdfandmaxBinsoptional (=None). When omitted, the wrapper substitutes Deequ's own defaults pulled from the JVM via the...$default$Naccessors — the same pattern PyDeequ already uses insatisfies():binningUdfdefault → ScalaOption.emptymaxBinsdefault →Histogram.MaximumAllowedDetailBinsPulling defaults from the live jar (rather than hardcoding) keeps them correct across Deequ versions. Fully backward-compatible: existing positional calls are unchanged.
Tests
Added to
tests/test_checks.py:test_hasNumberOfDistinctValues_without_binning_argstest_hasHistogramValues_without_binning_argsThe pre-existing positional-arg tests are left untouched to prove backward compatibility. Validated against real Spark 3.5 / Deequ 2.0.8 (8 passed). CI runs the full pyspark matrix.
Closes #81