feat: add Distance analyzer for categorical feature drift (L-infinity, chi-squared) (#164) - #276
nikolauspschuetz wants to merge 3 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Ready for review. Adds a |
8368609 to
57cdd69
Compare
|
Thanks for the automated review pass. The current revision addresses these findings:
Resolving the threads accordingly. |
|
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. |
8b6b1c9 to
bab6b38
Compare
|
rebased onto the latest master — green and mergeable. adds a Distance analyzer wrapper for categorical feature drift (L-infinity, chi-squared). still relevant on my end and happy to keep it current; would appreciate a review when there's bandwidth. |
|
@nikolauspschuetz Can you rebase on master so the updated test matrix runs on this? |
Expose Deequ's com.amazon.deequ.analyzers.Distance categorical distance
(L-infinity and chi-squared) in PyDeequ. Distance is a Scala object with
static-style methods rather than an Analyzer subclass, so it is wrapped as
a Distance helper class (not via addAnalyzer) that bridges two
{category: count} distributions to the JVM and returns the numeric distance.
- Add Distance class with categoricalDistance(distribution1, distribution2,
correctForLowNumberOfSamples, method, alpha, Yates/Cochran thresholds)
- Add CategoricalDistanceMethod enum (LInfinity, Chisquare)
- Build the required Scala mutable.Map[String, Long] via explicit Long-boxing
(java.lang.Long[] array -> genericWrapArray -> zip -> toMap), with NO Spark
job. Array element slots preserve Long boxing JVM-side and no value is read
back into Python, so the typing survives (py4j otherwise auto-unboxes per-
value Longs to int, triggering a ClassCastException in Deequ's e._2.toDouble).
Uses only core Scala 2.12 stdlib (no ambient Java->Scala implicits), so it is
portable across all supported Spark builds 3.1-3.5.
- Guard against empty distributions with a clear ValueError
- Document in docs/analyzers.md and add tests covering L-infinity, chi-squared,
single-category, empty-dict ValueError, and the invalid-method path
Numerical distance is intentionally out of scope: it requires a JVM
QuantileNonSample[Double] with no convenient Python construction path.
The alpha argument to categoricalDistance flows through LInfinityMethod(scala.Option.apply(alpha)) -- the only Option[Double] bridge in the Distance wrapper -- and was previously untested. Assert that two different alpha significance levels yield different distances, proving the value is genuinely consumed JVM-side.
bab6b38 to
4842cd9
Compare
| conforms = getattr(self._jvm.scala.Predef, "$conforms")() | ||
| immutable_map = zipped.toMap(conforms) | ||
|
|
||
| # Copy the immutable Scala Map[String, Long] into a mutable.HashMap, |
There was a problem hiding this comment.
BUG: The _to_scala_mutable_long_map method uses Scala 2.12-only APIs (Seq.canBuildFrom(), zip(seq, cbf), Predef.$conforms) that were removed in Scala 2.13. Since Spark 4.1 (now a supported target per the incremental diff to configs.py) ships with Scala 2.13, Distance.categoricalDistance will raise a Py4JError when invoked on the Spark 4.1 target.
configs.py incremental diff lines 9–12:
"4.1": "com.amazon.deequ:deequ:2.0.18-spark-4.1"added to SPARK_TO_DEEQU_COORD_MAPPING. analyzers.py line 919:can_build_from = self._jvm.scala.collection.Seq.canBuildFrom()(Scala 2.12 only). Line 920:keys_seq.zip(values_seq, can_build_from)(2-arg zip removed in 2.13). Line 921:getattr(self._jvm.scala.Predef, "$conforms")()(Scala 2.12 implicit). Spark 4.1 is built on Scala 2.13.
Refutation trail (why this survived the Critic's disprove pass)
Hypothesis (Investigator): _to_scala_mutable_long_map uses self._jvm.scala.collection.Seq.canBuildFrom() which was removed in Scala 2.13; the incremental diff adds Spark 4.1 to SPARK_TO_DEEQU_COORD_MAPPING, and Spark 4.1 ships Scala 2.13 where this API is unavailable.
Disprove attempt (Critic): Verified configs.py in incremental diff: lines 9-12 add "4.1": "com.amazon.deequ:deequ:2.0.18-spark-4.1" to SPARK_TO_DEEQU_COORD_MAPPING. The incremental diff also adds _to_scala_mutable_long_map at line 919 with can_build_from = self._jvm.scala.collection.Seq.canBuildFrom(), keys_seq.zip(values_seq, can_build_from), and getattr(self._jvm.scala.Predef, "$conforms")() — all Scala 2.12 APIs removed in Scala 2.13. The author's existing_feedback justification ("safe for 3.1–3.5, all Scala 2.12") relied on an assumption invalidated by the 4.1 target added in this same PR.
The Critic's default verdict is OVERTURNED. UPHELD findings are those it tried — and failed — to refute.
scala.collection.Seq.canBuildFrom was removed in Scala 2.13 and zip no longer takes an implicit CanBuildFrom, so the Distance tests failed on the Spark 4.1 matrix entry with Py4JError: scala.collection.Seq.canBuildFrom does not exist in the JVM Probe for canBuildFrom at runtime and fall back to the 2.13 zip signature, and accumulate the zipped pairs into mutable.HashMap with ++= rather than toMap(Predef.$conforms), which is deprecated in 2.13. The java.lang.Long[] array is retained: py4j converts a java.lang.Long it returns to Python into a Python int that re-enters the JVM as an Integer, which makes Deequ's unboxToLong throw ClassCastException. java.util.HashMap via JavaConverters and mutable.HashMap.put/update were both verified to fail for that reason on 2.12 and 2.13 alike. Verified on pyspark 3.5 (Scala 2.12.18, deequ 2.0.21-spark-3.5) and pyspark 4.1.2 (Scala 2.13.17, deequ 2.0.18-spark-4.1): 6 passed on both, with identical LInfinity and chi-squared values.
2 participants
Problem
Deequ's
Distanceanalyzer (com/amazon/deequ/analyzers/Distance.scala) computes distribution distance for feature-drift detection (L-infinity and chi-squared), but it was not exposed in PyDeequ. Requested in #164 (labeled good first issue / help wanted / feature request).What this adds
A
Distancehelper plus aCategoricalDistanceMethodenum exposing categorical distance:Supports both L-infinity and chi-squared, with the full alpha / Yates / Cochran parameters.
Design note (please scrutinize)
Deequ's
Distanceis a plain object with static-style methods, not anAnalyzersubclass, so it cannot go throughAnalysisRunBuilder.addAnalyzer(...).run(). I exposed it faithfully as a helper rather than faking analyzer integration.categoricalDistancerequires a Scalamutable.Map[String, Long]. py4j auto-unboxes individualjava.lang.Longback to Python ints (re-entering asInteger), which makes Deequ'se._2.toDoublethrowClassCastException. To keep values genuinelyLong-typed, each dict is round-tripped through a one-row Spark DataFrame with aMapType(StringType, LongType)column and collected JVM-side. Tradeoff: a small Spark job per distribution — acceptable for a drift helper, and the only reliable path found. Open to a lighter approach if reviewers prefer one.Scope: categorical distance only.
numericalDistancerequires constructing a JVMQuantileNonSample[Double]with no clean Python path and is intentionally left out (matches the issue).Tests
Added to
tests/test_analyzers.py:test_Distance_categorical_LInfinityandtest_Distance_categorical_Chisquare. Validated against the live Deequ 2.0.8 jar on Spark 3.5 (2 passed; full analyzer suite 27 passed / 22 pre-existing xfails, no regressions). Docs updated indocs/analyzers.md.Closes #164