show transformed data not dataop itself in fit report (#1623)

Author: jeromedockes

CHANGES.rst

@@ -50,6 +50,9 @@ Bugfixes
   (not into their subclasses). If you need the items evaluated (ie if they
   contain DataOps or Choices), store them in one of the builtin collections.
   :pr:`1612` by :user:`Jérôme Dockès <jeromedockes>`.
+- :meth:`SkrubLearner.report` with ``mode="fit"`` used to display the dataops
+  themselves, rather than their outputs, in the report. This has been fixed in
+  :pr:`1623` by :user:`Jérôme Dockès <jeromedockes>`.
 - Fixed a bug that happened when ``get_feature_names_out`` was called on instances
   of the :class:`DatetimeEncoder`. :pr:`1622` by :user:`Riccardo Cappuzzo<rcap107>`.
 

skrub/_data_ops/_evaluation.py

@@ -444,10 +444,9 @@ def evaluate(data_op, mode="preview", environment=None, clear=False, callbacks=(
     else:
         callbacks = ()
     try:
-        result = _Evaluator(
-            mode=mode, environment=environment, callbacks=callbacks
-        ).run(data_op)
-        return data_op if mode == "fit" else result
+        return _Evaluator(mode=mode, environment=environment, callbacks=callbacks).run(
+            data_op
+        )
     finally:
         if clear:
             clear_results(data_op, mode=mode)

skrub/_data_ops/tests/test_evaluation.py

@@ -382,3 +382,12 @@ class Dict(dict):
 
     d = Dict()
     assert skrub.as_data_op(d).skb.eval() is d
+
+
+def test_eval_fit():
+    # evaluate() does no special handling of "fit" mode.
+    # the learner's fit() discards the result and returns self.
+    dop = skrub.as_data_op(1)
+    assert _evaluation.evaluate(dop, mode="fit") == 1
+    learner = dop.skb.make_learner()
+    assert learner.fit({}) is learner

skrub/_data_ops/tests/test_inspection.py

@@ -2,6 +2,7 @@
 import re
 import sys
 import webbrowser
+from pathlib import Path
 from unittest.mock import Mock
 
 import pytest
@@ -84,6 +85,25 @@ def test_full_report_failed_apply():
     assert report["error"] is not None
 
 
+@pytest.mark.skipif(not _inspection._has_graphviz(), reason="report requires graphviz")
+def test_report_fit_mode():
+    # non-regression: in fit mode the individual node pages used to show the
+    # dataop itself as the output instead of the result of fit_transform for
+    # intermediate nodes.
+    learner = (
+        skrub.var("a")
+        .skb.apply_func(lambda x: x[::-1])
+        .skb.apply_func(lambda x: x.upper())
+        .skb.make_learner()
+    )
+    report = learner.report(environment={"a": "hello"}, mode="fit", open=False)
+    report_dir = Path(report["report_path"]).parent
+    node_1_report = (report_dir / "node_1.html").read_text("utf-8")
+    # the page does display the actual output rather than the repr of the node
+    # ('hello'[::-1] = 'olleh')
+    assert "olleh" in node_1_report
+
+
 @pytest.mark.skipif(not _inspection._has_graphviz(), reason="report requires graphviz")
 def test_full_report_open(monkeypatch):
     mock = Mock()