feat: Introduce new summary section for data metrics by MoritzPotthoffQC · Pull Request #47 · Quantco/diffly

Moritz Potthoff (MoritzPotthoffQC) · 2026-07-17T12:01:53Z

Motivation

See this discussion: We want to display data metrics in their own section in the summary.

Changes

Add differentiation for the metrics
Add new section Data Inspection that gets all the data metrics

codecov · 2026-07-17T12:02:59Z

Codecov Report

❌ Patch coverage is 98.01325% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.64%. Comparing base (01cef28) to head (7ecd908).

Files with missing lines	Patch %	Lines
diffly/summary.py	97.91%	2 Missing ⚠️
diffly/comparison.py	90.90%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##              main      #47      +/-   ##
===========================================
- Coverage   100.00%   99.64%   -0.36%     
===========================================
  Files           14       14              
  Lines         1052     1134      +82     
===========================================
+ Hits          1052     1130      +78     
- Misses           0        4       +4

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR extends Diffly’s summary reporting to distinguish change metrics (computed over deltas) from data inspection metrics (describing left/right datasets), and renders the latter in a new dedicated “Data Inspection” section while also separating them in the JSON schema.

Changes:

Add Metric.kind ("change" vs "data") and mark the Null% preset as a "data" metric.
Split per-column metric outputs into metrics vs data_metrics in the summary JSON, and render "data" metrics in a new “Data Inspection” section.
Update summary tests and regenerate pretty-output fixtures to reflect the new section/layout.

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
diffly/metrics/_common.py	Adds `Metric.kind` to classify metrics by rendering section.
diffly/metrics/data.py	Marks `Null%` preset as a `"data"` metric.
diffly/summary.py	Routes metric labels into change vs data lists; adds “Data Inspection” rendering and JSON schema changes.
tests/summary/test_summary.py	Adds coverage for `metrics` vs `data_metrics` JSON routing; updates expected JSON shape.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_True_slim_True_sample_rows_True_sample_pk_True.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_True_slim_True_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_True_slim_False_sample_rows_True_sample_pk_True.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_True_slim_False_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_False_slim_True_sample_rows_True_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_False_slim_True_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_False_slim_False_sample_rows_True_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_False_slim_False_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_True_slim_True_sample_rows_True_sample_pk_True.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_True_slim_True_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_True_slim_False_sample_rows_True_sample_pk_True.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_True_slim_False_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_False_slim_True_sample_rows_True_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_False_slim_True_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_False_slim_False_sample_rows_True_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_False_slim_False_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_True_slim_True_sample_rows_True_sample_pk_True.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_True_slim_True_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_True_slim_False_sample_rows_True_sample_pk_True.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_True_slim_False_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_False_slim_True_sample_rows_True_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_False_slim_True_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_False_slim_False_sample_rows_True_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_False_slim_False_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_True_slim_True_sample_rows_True_sample_pk_True.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_True_slim_True_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_True_slim_False_sample_rows_True_sample_pk_True.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_True_slim_False_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_False_slim_True_sample_rows_True_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_False_slim_True_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_False_slim_False_sample_rows_True_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.
tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_False_slim_False_sample_rows_False_sample_pk_False.txt	Updates fixture output to move `Null%` into “Data Inspection”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

EgeKaraismailogluQC

Thank you Moritz Potthoff (@MoritzPotthoffQC)! In general I am happy with the layout. I added a few comments. In addition to those, I think it would be nice to have at least one summary fixture where we render two or more data metrics rather than just one, just to see what that looks like (maybe I missed it?).

EgeKaraismailogluQC · 2026-07-22T16:15:53Z


    fn: MetricFn
    selector: cs.Selector
+    kind: Literal["change", "data"] = "change"


I think it could be good to not provide a default here to force the user to think about what kind of metric they implemented. Wdyt?

Also, nothing prevents me from "registering" a change metric as a data metric and vice versa. Going a step further, I wish there was a way to infer kind from the type of fn automatically. Right now that does not work of course since both change metrics and data metrics are the same type. Something along these lines:

DataMetricFn = Callable[[pl.Expr], pl.Expr]

and then rewrite:

def null_fraction_change(col: pl.Expr) -> pl.Expr: return col.is_null().mean() class Metric: fn: ChangeMetricFn | DataMetricFn #(property) def kind(self) -> str: return "data" if isinstance(DataMetricFn, self.fn) else "change"

Metric would then also need to wrap .fn if it is a DataMetricFn to make it have the same interface. Could you follow my idea? Wdyt?

Agreed, thanks for the good point!

I went even a bit further now and clearly split the metrics by introducing two classes ChangeMetric and Data Metric. That way, the diff between them is clear and DataMetric even has a bit more logic. Internally, we now always use these classes (rather then bare callables), because I find it intuitive that way. When running a comparison, the user can still pass in callables. These will be wrapped in the appropriate class, based on the arity of the callable (one expression -> data, two expressions -> change)

One more change is that we now properly compute data metrics on both columns independently. Previously, we evaluated them on the joined rows (only). Now, we actually compute them on both columns independently. So now we also consider unjoined rows for the data metrics (which is what I expect here).

Copilot

Pull request overview

Copilot reviewed 51 out of 52 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 84 out of 85 changed files in this pull request and generated 3 comments.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Moritz Potthoff (MoritzPotthoffQC) · 2026-07-23T15:04:14Z

EgeKaraismailogluQC I made quite a few changes since your review. I'd appreciate another look. Let me know if you want to chat about this!

EgeKaraismailogluQC

Thank you for working on this Moritz Potthoff (@MoritzPotthoffQC), looks very nice! Just a few cosmetic comments. Let's also add ! to the PR title to mark it as breaking.

EgeKaraismailogluQC · 2026-07-24T08:00:28Z

-The expressions refer to the left-side and right-side values of a single column across
-all joined rows.
-"""
+ALL_METRICS: dict[str, Metric] = {


?

Suggested change

ALL_METRICS: dict[str, Metric] = {

DEFAULT_METRICS: dict[str, Metric] = {

EgeKaraismailogluQC · 2026-07-24T08:05:36Z

+DEFAULT_DATA_METRICS: dict[str, DataMetric] = {
+    "Null%": DataMetric(
+        fn=null_fraction,
+        selector=cs.all(),


It is the default

Suggested change

selector=cs.all(),

EgeKaraismailogluQC · 2026-07-24T08:09:35Z

+
+
+def _resolve_metric(v: ChangeMetricFn | DataMetricFn | Metric) -> Metric:
+    if isinstance(v, (ChangeMetric, DataMetric)):


?

Suggested change

if isinstance(v, (ChangeMetric, DataMetric)):

if isinstance(v, Metric):

EgeKaraismailogluQC · 2026-07-24T08:16:03Z

+        return v
+    # Infer the metric family from the callable's arity: a single-argument
+    # callable describes one side (data), two arguments describe a change.
+    if len(inspect.signature(v).parameters) >= 2:


Why not:

Suggested change

if len(inspect.signature(v).parameters) >= 2:

if isinstance(v, ChangeMetricFn):

Also, how about being a bit defensive here and do:

elif isinstance(v, DataMetricFn): ... else: raise

so that the signature issue is easier to understand for the user?

EgeKaraismailogluQC · 2026-07-24T08:58:05Z

                n_total_changes=n_total_changes,
                changes=changes,
-                metrics=metrics_by_column.get(col_name),
+                change_metrics=change_metrics_by_column.get(col_name) or None,


?

Suggested change

change_metrics=change_metrics_by_column.get(col_name) or None,

change_metrics=change_metrics_by_column.get(col_name),

EgeKaraismailogluQC · 2026-07-24T08:59:54Z

@@ -963,20 +1041,50 @@ def select_columns(selector: cs.Selector) -> set[str]:
        for label, metric in metrics.items()
        for column in sorted(metric_to_columns[label])
    ]
-    row = joined.select(agg_exprs).collect().row(0, named=True)
-    for label, columns in metric_to_columns.items():
-        for column in columns:
+
+    row = comp.joined(lazy=True).select(agg_exprs).collect().row(0, named=True)
+    out: dict[str, dict[str, Any]] = {
+        c: {} for c in set().union(*metric_to_columns.values())
+    }
+    for label in metrics:
+        for column in metric_to_columns[label]:
            out[column][label] = row[f"{label}__{column}"]
    return out


Am I missing something or is this part just a "shallow" refactor?

EgeKaraismailogluQC · 2026-07-24T09:03:04Z

+        formatter=lambda value: f"{round(value * 100, 2)}%",
+        delta_formatter=lambda value: f"{round(value * 100, 2)}",


Should we use these as the default formatter for DataMetric?

EgeKaraismailogluQC · 2026-07-24T09:04:48Z


-from collections.abc import Callable
-from dataclasses import dataclass
+Metric = ChangeMetric | DataMetric


We use ChangeMetricFn | DataMetricFn in a few places. For comfort, how about also defining

MetricFn = ChangeMetricFn | DataMetricFn

?

init

bff2594

Moritz Potthoff (MoritzPotthoffQC) self-assigned this Jul 17, 2026

github-actions Bot added the enhancement New feature or request label Jul 17, 2026

add test

51fbda2

Moritz Potthoff (MoritzPotthoffQC) requested a review from Copilot July 17, 2026 12:09

Copilot started reviewing on behalf of Moritz Potthoff (MoritzPotthoffQC) July 17, 2026 12:09 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

Comment thread diffly/summary.py Outdated

Moritz Potthoff (MoritzPotthoffQC) added 3 commits July 17, 2026 14:39

refactor

cb086ef

simplify

526c944

fix

5732c37

Moritz Potthoff (MoritzPotthoffQC) marked this pull request as ready for review July 17, 2026 13:56

Moritz Potthoff (MoritzPotthoffQC) requested review from EgeKaraismailogluQC, Marius Merkle (MariusMerkleQC) and Oliver Borchert (borchero) as code owners July 17, 2026 13:56

Moritz Potthoff (MoritzPotthoffQC) mentioned this pull request Jul 17, 2026

feat: Add data metrics for numeric columns #46

Draft

EgeKaraismailogluQC reviewed Jul 22, 2026

View reviewed changes

Moritz Potthoff (MoritzPotthoffQC) added 2 commits July 22, 2026 19:07

review

5867021

review

6b705e7

Moritz Potthoff (MoritzPotthoffQC) marked this pull request as draft July 22, 2026 17:29

Moritz Potthoff (MoritzPotthoffQC) added 7 commits July 22, 2026 19:48

review

6c868d6

review

6802592

refactor

d827967

simplify

2a49d96

simplify

eef3d3b

simplify

e918bbf

fix

65d80d6

Moritz Potthoff (MoritzPotthoffQC) requested a review from Copilot July 23, 2026 12:35

Copilot started reviewing on behalf of Moritz Potthoff (MoritzPotthoffQC) July 23, 2026 12:36 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread diffly/metrics/__init__.py Outdated

Moritz Potthoff (MoritzPotthoffQC) added 9 commits July 23, 2026 14:52

cleanup

ffeb05a

improve

c116fdf

docs

a57dc0f

review

1adcf78

fix

4000181

simplify

a52e24c

fix

2d3f152

fix

a0955c0

add test

0d9be70

Moritz Potthoff (MoritzPotthoffQC) marked this pull request as ready for review July 23, 2026 14:56

Moritz Potthoff (MoritzPotthoffQC) requested a review from Copilot July 23, 2026 14:56

Copilot started reviewing on behalf of Moritz Potthoff (MoritzPotthoffQC) July 23, 2026 14:57 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread diffly/comparison.py

Comment thread diffly/cli.py

Comment thread diffly/metrics/__init__.py Outdated

Potential fix for pull request finding

7ecd908

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

EgeKaraismailogluQC reviewed Jul 24, 2026

View reviewed changes

	ALL_METRICS: dict[str, Metric] = {
	DEFAULT_METRICS: dict[str, Metric] = {



		def _resolve_metric(v: ChangeMetricFn \| DataMetricFn \| Metric) -> Metric:
		if isinstance(v, (ChangeMetric, DataMetric)):

	if isinstance(v, (ChangeMetric, DataMetric)):
	if isinstance(v, Metric):

	if len(inspect.signature(v).parameters) >= 2:
	if isinstance(v, ChangeMetricFn):

	change_metrics=change_metrics_by_column.get(col_name) or None,
	change_metrics=change_metrics_by_column.get(col_name),

		formatter=lambda value: f"{round(value * 100, 2)}%",
		delta_formatter=lambda value: f"{round(value * 100, 2)}",

Uh oh!

Conversation

Moritz Potthoff (MoritzPotthoffQC) commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Uh oh!

codecov Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

EgeKaraismailogluQC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Moritz Potthoff (MoritzPotthoffQC) commented Jul 23, 2026

Uh oh!

EgeKaraismailogluQC left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Moritz Potthoff (MoritzPotthoffQC) commented Jul 17, 2026 •

edited

Loading

codecov Bot commented Jul 17, 2026 •

edited

Loading