fix(metrics): GroupBy(ValueDrift) widget keeps decorated display name (#1706)#1876
Open
jbbqqf wants to merge 1 commit into
Open
fix(metrics): GroupBy(ValueDrift) widget keeps decorated display name (#1706)#1876jbbqqf wants to merge 1 commit into
jbbqqf wants to merge 1 commit into
Conversation
…evidentlyai#1706) ValueDriftCalculation._render hard-coded the widget title as "Drift in column 'X'", ignoring whatever display_name had been patched onto the calculation. As a result, GroupBy(ValueDrift(...)) widgets always rendered the bare column name and dropped the "group by '<col>' for label: '<label>'" suffix that GroupBy adds for every other metric (MaxValue/MeanValue/etc which go through StatisticsCalculation.calculate and use self.display_name() at render time). Pass display_name through to _render and use it as the widget header when it carries the GroupBy decoration; keep the historical wording otherwise so non-GroupBy reports are unchanged. Add a regression test that walks the rendered widget params for both shapes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
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
Closes #1706.
`ValueDriftCalculation._render` hard-coded the widget title as
`f"Drift in column '{result.column_name}'"` (see
`src/evidently/metrics/column_statistics.py`). When the calculation is
wrapped in `GroupBy(...)`, `GroupByMetricCalculation.calculate` patches
`self.calculation.display_name` so that other metrics (MaxValue, MeanValue,
…) pick up the "group by '<col>' for label: '<label>'" suffix at render
time — they call `self.display_name()` from `StatisticsCalculation.calculate`.
ValueDrift never read the patched name, so its widget always rendered the
bare column name regardless of how it was wrapped, which is exactly the
bug reported in #1706.
Fix
`src/evidently/metrics/column_statistics.py`:
when it carries the GroupBy suffix (`" group by '"` substring). This
preserves the historical "Drift in column 'X'" wording for non-GroupBy
reports — no surprise behavior change for existing users.
the counter header.
Reproduce BEFORE/AFTER yourself (copy-paste)
```bash
git fetch origin && git fetch https://github.com/jbbqqf/evidently.git fix/1706-groupby-valuedrift-display-name:_pr1706
pip install -q -e ".[dev]" >/dev/null
run_check() {
python - <<'PY'
import numpy as np, pandas as pd
from evidently.core.datasets import Dataset, DataDefinition
from evidently.core.report import Report
from evidently.metrics.column_statistics import ValueDrift
from evidently.metrics.group_by import GroupBy
rng = np.random.default_rng(0)
df = pd.DataFrame({
"value": np.concatenate([rng.normal(size=100), rng.normal(loc=2, size=100)]),
"group": ["a"]*100 + ["b"]*100,
})
ref = Dataset.from_pandas(df, data_definition=DataDefinition())
cur = Dataset.from_pandas(df.copy(), data_definition=DataDefinition())
snap = Report([GroupBy(ValueDrift(column="value"), "group")]).run(cur, ref)
ctx = snap.context
for fp in snap.metric_results:
res = ctx.get_metric_result(fp)
widgets = res.widget if isinstance(res.widget, list) else [res.widget]
for w in widgets:
for c in (w.dict().get("params") or {}).get("counters") or []:
print("widget title:", c.get("value"))
PY
}
BEFORE — origin/main: every counter says 'Drift in column 'value'', no group/label suffix.
git checkout origin/main -- src/evidently/metrics/column_statistics.py
run_check
Expected: 'widget title: Drift in column \'value\'' (twice).
AFTER — this branch: each widget shows the GroupBy decoration.
git checkout _pr1706 -- src/evidently/metrics/column_statistics.py
run_check
Expected: 'widget title: Value drift for value group by \'group\' for label: \'a\'' and same for label 'b'.
git checkout origin/main -- src/evidently/metrics/column_statistics.py
```
What I ran locally
`origin/main` the GroupBy assertion fails with the bare "Drift in column 'value'"
title.
Edge cases
AI disclosure
This pull request was authored with assistance from Anthropic's Claude (an AI
coding assistant) running under my direction. I traced the patching mechanism
(`metrics/group_by.py:_patched_display_name`) and confirmed the fix matches
the convention StatisticsCalculation already uses.