From 4f4ae77cfde73a1fbf20898e1368a07f027766ed Mon Sep 17 00:00:00 2001 From: Riccardo Cappuzzo Date: Fri, 12 Sep 2025 16:41:04 +0200 Subject: [PATCH 1/6] adding check on type --- skrub/_dataframe/_common.py | 8 ++++++++ skrub/_dataframe/tests/test_common.py | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/skrub/_dataframe/_common.py b/skrub/_dataframe/_common.py index c3725dee0..d635e0149 100644 --- a/skrub/_dataframe/_common.py +++ b/skrub/_dataframe/_common.py @@ -119,6 +119,14 @@ def _raise(obj, kind="object"): + from .._data_ops._data_ops import DataOp + + if isinstance(obj, DataOp): + raise TypeError( + "Operation not supported on DataOps. Expecting a Pandas or Polars " + f"{kind}, but got a DataOp. You may want to use `.skb.eval()` to " + "evaluate the DataOp and obtain a dataframe." + ) raise TypeError( "Operation not supported on this object. Expecting a Pandas or Polars " f"{kind}, but got an object of type {type(obj)}." diff --git a/skrub/_dataframe/tests/test_common.py b/skrub/_dataframe/tests/test_common.py index 2b1fe6a88..60a9e07c7 100644 --- a/skrub/_dataframe/tests/test_common.py +++ b/skrub/_dataframe/tests/test_common.py @@ -15,6 +15,7 @@ from numpy.testing import assert_array_equal from pandas.testing import assert_frame_equal as pd_assert_frame_equal +import skrub from skrub import selectors as s from skrub._dataframe import _common as ns @@ -40,6 +41,12 @@ def test_not_implemented(): params = [None] * n_params with pytest.raises(TypeError): func(*params) + dop = [skrub.var("a")] * n_params + with pytest.raises( + TypeError, + match=r"Operation not supported on DataOps.*Expecting a Pandas*", + ): + func(*dop) # From 8687783bf197917c62748c4ecd27f66b7d712d61 Mon Sep 17 00:00:00 2001 From: Riccardo Cappuzzo Date: Fri, 12 Sep 2025 16:49:17 +0200 Subject: [PATCH 2/6] changelog --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 4f84c53bb..f41a51978 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -16,6 +16,8 @@ Changes removed in the next release of skrub. :pr:`1546` by :user:`Vincent Maladiere `. +- Improved error messages when a DataOp is being sent to dispatched functions. + :pr:`1607` by :user:`Riccardo Cappuzzo`. Bugfixes -------- From db659eba54e633de3bbc1cfe81ab75a2d4c908b3 Mon Sep 17 00:00:00 2001 From: Riccardo Cappuzzo <7548232+rcap107@users.noreply.github.com> Date: Tue, 16 Sep 2025 16:30:43 +0200 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Gael Varoquaux --- skrub/_dataframe/_common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/skrub/_dataframe/_common.py b/skrub/_dataframe/_common.py index d635e0149..f51d5c984 100644 --- a/skrub/_dataframe/_common.py +++ b/skrub/_dataframe/_common.py @@ -123,9 +123,9 @@ def _raise(obj, kind="object"): if isinstance(obj, DataOp): raise TypeError( - "Operation not supported on DataOps. Expecting a Pandas or Polars " + "This operation is not supported on DataOps. It Expects a Pandas or Polars " f"{kind}, but got a DataOp. You may want to use `.skb.eval()` to " - "evaluate the DataOp and obtain a dataframe." + "evaluate the DataOp into a dataframe and run your operation on it." ) raise TypeError( "Operation not supported on this object. Expecting a Pandas or Polars " From d6f55a52def63e4075ca77ca628cbf483c423aaa Mon Sep 17 00:00:00 2001 From: Riccardo Cappuzzo Date: Tue, 16 Sep 2025 16:34:08 +0200 Subject: [PATCH 4/6] fixing test --- skrub/_dataframe/_common.py | 2 +- skrub/_dataframe/tests/test_common.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/skrub/_dataframe/_common.py b/skrub/_dataframe/_common.py index f51d5c984..12866e689 100644 --- a/skrub/_dataframe/_common.py +++ b/skrub/_dataframe/_common.py @@ -123,7 +123,7 @@ def _raise(obj, kind="object"): if isinstance(obj, DataOp): raise TypeError( - "This operation is not supported on DataOps. It Expects a Pandas or Polars " + "This operation is not supported on DataOps. It expects a Pandas or Polars " f"{kind}, but got a DataOp. You may want to use `.skb.eval()` to " "evaluate the DataOp into a dataframe and run your operation on it." ) diff --git a/skrub/_dataframe/tests/test_common.py b/skrub/_dataframe/tests/test_common.py index 60a9e07c7..acae825bb 100644 --- a/skrub/_dataframe/tests/test_common.py +++ b/skrub/_dataframe/tests/test_common.py @@ -44,7 +44,7 @@ def test_not_implemented(): dop = [skrub.var("a")] * n_params with pytest.raises( TypeError, - match=r"Operation not supported on DataOps.*Expecting a Pandas*", + match=r"This operation is not supported on DataOps..*It expects a Pandas*", ): func(*dop) From e0ff0486ca109e800f6e58ca53d0f102370daf56 Mon Sep 17 00:00:00 2001 From: Riccardo Cappuzzo Date: Wed, 17 Sep 2025 14:37:01 +0200 Subject: [PATCH 5/6] reworking message --- skrub/_dataframe/_common.py | 10 +++++++--- skrub/_dataframe/tests/test_common.py | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/skrub/_dataframe/_common.py b/skrub/_dataframe/_common.py index 12866e689..471639a2f 100644 --- a/skrub/_dataframe/_common.py +++ b/skrub/_dataframe/_common.py @@ -123,9 +123,13 @@ def _raise(obj, kind="object"): if isinstance(obj, DataOp): raise TypeError( - "This operation is not supported on DataOps. It expects a Pandas or Polars " - f"{kind}, but got a DataOp. You may want to use `.skb.eval()` to " - "evaluate the DataOp into a dataframe and run your operation on it." + "Expected a Pandas or Polars DataFrame, but got a skrub DataOp. " + "A function that expects an actual value cannot be applied directly" + " to a DataOp; you may want to (i) use op.skb.eval() or op.skb.preview()" + "to evaluate the dataop and turn it into an actual value or " + "(ii) use op.skb.apply_func or op.skb.apply to schedule the operation" + " for later execution (when the dataop is evaluated) " + "rather than computing it immediately." ) raise TypeError( "Operation not supported on this object. Expecting a Pandas or Polars " diff --git a/skrub/_dataframe/tests/test_common.py b/skrub/_dataframe/tests/test_common.py index acae825bb..b0db4de4f 100644 --- a/skrub/_dataframe/tests/test_common.py +++ b/skrub/_dataframe/tests/test_common.py @@ -44,7 +44,7 @@ def test_not_implemented(): dop = [skrub.var("a")] * n_params with pytest.raises( TypeError, - match=r"This operation is not supported on DataOps..*It expects a Pandas*", + match=r"Expected a Pandas or Polars DataFrame, but got a skrub DataOp", ): func(*dop) From 029fd5ff1eb318d86acd3e361a224e43b1762fab Mon Sep 17 00:00:00 2001 From: Riccardo Cappuzzo <7548232+rcap107@users.noreply.github.com> Date: Thu, 18 Sep 2025 09:47:16 +0200 Subject: [PATCH 6/6] Update skrub/_dataframe/_common.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jérôme Dockès --- skrub/_dataframe/_common.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/skrub/_dataframe/_common.py b/skrub/_dataframe/_common.py index 471639a2f..85272eac7 100644 --- a/skrub/_dataframe/_common.py +++ b/skrub/_dataframe/_common.py @@ -123,13 +123,13 @@ def _raise(obj, kind="object"): if isinstance(obj, DataOp): raise TypeError( - "Expected a Pandas or Polars DataFrame, but got a skrub DataOp. " - "A function that expects an actual value cannot be applied directly" - " to a DataOp; you may want to (i) use op.skb.eval() or op.skb.preview()" - "to evaluate the dataop and turn it into an actual value or " - "(ii) use op.skb.apply_func or op.skb.apply to schedule the operation" - " for later execution (when the dataop is evaluated) " - "rather than computing it immediately." + """Expected a Pandas or Polars DataFrame, but got a skrub DataOp. +A function that expects an actual value cannot be applied directly to a DataOp; +you may want to (i) use op.skb.eval() or op.skb.preview() to evaluate the +dataop and turn it into an actual value or (ii) use op.skb.apply_func() or +op.skb.apply() to schedule the operation for later execution (when the dataop is +evaluated) rather than computing it immediately. + """ ) raise TypeError( "Operation not supported on this object. Expecting a Pandas or Polars "