Skip to content

Commit 693bff1

Browse files
authored
check if missing method exists in .skb namespace (#1541)
1 parent a3ef99a commit 693bff1

2 files changed

Lines changed: 37 additions & 12 deletions

File tree

skrub/_data_ops/_data_ops.py

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,21 +1326,22 @@ def compute(self, e, mode, environment):
13261326
return self.target._skrub_impl.estimator_
13271327

13281328

1329+
def _checked_get_attr(obj, obj_value, attr_name):
1330+
try:
1331+
return getattr(obj_value, attr_name)
1332+
except AttributeError:
1333+
if isinstance(obj, DataOp) and hasattr(obj.skb, attr_name):
1334+
comment = f"Did you mean `.skb.{attr_name}` and forget the `.skb`?"
1335+
else:
1336+
comment = None
1337+
attribute_error(obj_value, attr_name, comment)
1338+
1339+
13291340
class GetAttr(DataOpImpl):
13301341
_fields = ["source_object", "attr_name"]
13311342

13321343
def compute(self, e, mode, environment):
1333-
try:
1334-
return getattr(e.source_object, e.attr_name)
1335-
except AttributeError:
1336-
pass
1337-
if isinstance(self.source_object, DataOp) and hasattr(
1338-
self.source_object.skb, e.attr_name
1339-
):
1340-
comment = f"Did you mean `.skb.{e.attr_name}` and forget the `.skb`?"
1341-
else:
1342-
comment = None
1343-
attribute_error(e.source_object, e.attr_name, comment)
1344+
return _checked_get_attr(self.source_object, e.source_object, e.attr_name)
13441345

13451346
def __repr__(self):
13461347
return f"<{self.__class__.__name__} {short_repr(self.attr_name)}>"
@@ -1421,8 +1422,9 @@ class CallMethod(DataOpImpl):
14211422
_fields = ["obj", "method_name", "args", "kwargs"]
14221423

14231424
def compute(self, e, mode, environment):
1425+
method = _checked_get_attr(self.obj, e.obj, e.method_name)
14241426
try:
1425-
return getattr(e.obj, e.method_name)(*e.args, **e.kwargs)
1427+
return method(*e.args, **e.kwargs)
14261428
except Exception as err:
14271429
# Better error message if we used the pandas DataFrame's `apply()`
14281430
# but we meant `.skb.apply()`

skrub/_data_ops/tests/test_errors.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import pickle
22
import re
3+
import traceback
34

45
import numpy as np
56
import pandas as pd
@@ -406,6 +407,28 @@ def test_attribute_errors():
406407
skrub.X().skb.applied_estimator
407408

408409

410+
def test_call_method_errors():
411+
# In this case we have a preview value, so the access to `mark_as_X` fails
412+
# right away, before we add the call (when creating a GetAttr node)
413+
a = skrub.var("a", 0)
414+
with pytest.raises(
415+
(AttributeError, RuntimeError),
416+
match=r"(?sm).*^Did you mean `\.skb\.mark_as_X`",
417+
):
418+
a.mark_as_X()
419+
a = skrub.var("a").mark_as_X()
420+
421+
# In this case we have no preview value so the error only happens when we
422+
# call `.skb.eval()` and provide a value. By then the GetAttr + Call have
423+
# been collapsed in the graph into a CallMethod node, so the error
424+
# originates from a different place but it should provide the same info.
425+
with pytest.raises((AttributeError, RuntimeError)) as exc:
426+
a.skb.eval({"a": 0})
427+
assert "Did you mean `.skb.mark_as_X` and forget the `.skb`?" in "\n".join(
428+
traceback.format_exception(exc.value, exc.value, exc.tb)
429+
)
430+
431+
409432
def test_concat_horizontal_numpy():
410433
a = skrub.var("a", skrub.datasets.toy_orders().orders)
411434
b = skrub.var("b", np.eye(3))

0 commit comments

Comments
 (0)