Skip to content

Commit

Permalink
chore: Consistent invalid IntoExpr error (#1379)
Browse files Browse the repository at this point in the history

---------

Co-authored-by: Marco Gorelli <[email protected]>
  • Loading branch information
EdAbati and MarcoGorelli authored Nov 17, 2024
1 parent 0a5c8b9 commit 19df320
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 18 deletions.
6 changes: 3 additions & 3 deletions narwhals/_dask/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from typing import TYPE_CHECKING
from typing import Any

from narwhals._exceptions import InvalidIntoExprError
from narwhals.dependencies import get_pandas
from narwhals.dependencies import get_polars
from narwhals.dependencies import get_pyarrow
Expand Down Expand Up @@ -44,9 +45,8 @@ def parse_exprs_and_named_exprs(
_results = expr._call(df)
elif isinstance(expr, str):
_results = [df._native_frame[expr]]
else: # pragma: no cover
msg = f"Expected expression or column name, got: {expr}"
raise TypeError(msg)
else:
raise InvalidIntoExprError.from_invalid_type(type(expr))
return_scalar = getattr(expr, "_returns_scalar", False)
for _result in _results:
results[_result.name] = _result[0] if return_scalar else _result
Expand Down
16 changes: 16 additions & 0 deletions narwhals/_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,19 @@ def from_missing_and_available_column_names(


class InvalidOperationError(Exception): ...


class InvalidIntoExprError(TypeError):
def __init__(self, message: str) -> None:
self.message = message
super().__init__(self.message)

@classmethod
def from_invalid_type(cls, invalid_type: type) -> InvalidIntoExprError:
message = (
f"Expected an object which can be converted into an expression, got {invalid_type}\n\n"
"Hint: if you were trying to select a column which does not have a string column name, then "
"you should explicitly use `nw.col`.\nFor example, `df.select(nw.col(0))` if you have a column "
"named `0`."
)
return InvalidIntoExprError(message)
9 changes: 2 additions & 7 deletions narwhals/_expression_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from typing import cast
from typing import overload

from narwhals._exceptions import InvalidIntoExprError
from narwhals.dependencies import is_numpy_array

if TYPE_CHECKING:
Expand Down Expand Up @@ -190,13 +191,7 @@ def parse_into_expr(
if is_numpy_array(into_expr):
series = namespace._create_compliant_series(into_expr)
return namespace._create_expr_from_series(series) # type: ignore[arg-type]
msg = (
f"Expected an object which can be converted into an expression, got {type(into_expr)}\n\n" # pragma: no cover
"Hint: if you were trying to select a column which does not have a string column name, then "
"you should explicitly use `nw.col`.\nFor example, `df.select(nw.col(0))` if you have a column "
"named `0`."
)
raise TypeError(msg)
raise InvalidIntoExprError.from_invalid_type(type(into_expr))


def reuse_series_implementation(
Expand Down
35 changes: 29 additions & 6 deletions narwhals/_polars/dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import Sequence

from narwhals._exceptions import ColumnNotFoundError
from narwhals._exceptions import InvalidIntoExprError
from narwhals._polars.namespace import PolarsNamespace
from narwhals._polars.utils import convert_str_slice_to_int_slice
from narwhals._polars.utils import extract_args_kwargs
Expand Down Expand Up @@ -88,9 +89,18 @@ def func(*args: Any, **kwargs: Any) -> Any:
getattr(self._native_frame, attr)(*args, **kwargs)
)
except pl.exceptions.ColumnNotFoundError as e:
msg = str(e)
msg += f"\n\nHint: Did you mean one of these columns: {self.columns}?"
raise ColumnNotFoundError(str(e)) from e
msg = (
f"t {e!s}\n\nHint: Did you mean one of these columns: {self.columns}?"
)
raise ColumnNotFoundError(msg) from e
except TypeError as e:
e_str = str(e)
if (
"cannot create expression literal" in e_str
or "invalid literal" in e_str
):
raise InvalidIntoExprError(e_str) from e
raise

return func

Expand Down Expand Up @@ -315,10 +325,23 @@ def _from_native_frame(self, df: Any) -> Self:

def __getattr__(self, attr: str) -> Any:
def func(*args: Any, **kwargs: Any) -> Any:
import polars as pl # ignore-banned-import

args, kwargs = extract_args_kwargs(args, kwargs) # type: ignore[assignment]
return self._from_native_frame(
getattr(self._native_frame, attr)(*args, **kwargs)
)
try:
return self._from_native_frame(
getattr(self._native_frame, attr)(*args, **kwargs)
)
except pl.exceptions.ColumnNotFoundError as e: # pragma: no cover
raise ColumnNotFoundError(str(e)) from e
except TypeError as e:
e_str = str(e)
if (
"cannot create expression literal" in e_str
or "invalid literal" in e_str
):
raise InvalidIntoExprError(e_str) from e
raise

return func

Expand Down
21 changes: 19 additions & 2 deletions tests/frame/select_test.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
from __future__ import annotations

from typing import Any

import pandas as pd
import pyarrow as pa
import pytest

import narwhals.stable.v1 as nw
from narwhals._exceptions import ColumnNotFoundError
from narwhals._exceptions import InvalidIntoExprError
from tests.utils import PANDAS_VERSION
from tests.utils import POLARS_VERSION
from tests.utils import Constructor
from tests.utils import assert_equal_data


class Foo: ...


def test_select(constructor: Constructor) -> None:
data = {"a": [1, 3, 2], "b": [4, 4, 6], "z": [7.0, 8, 9]}
df = nw.from_native(constructor(data))
Expand All @@ -32,12 +38,23 @@ def test_non_string_select() -> None:
pd.testing.assert_frame_equal(result, expected)


def test_non_string_select_invalid() -> None:
def test_int_select_pandas() -> None:
df = nw.from_native(pd.DataFrame({0: [1, 2], "b": [3, 4]}))
with pytest.raises(TypeError, match="\n\nHint: if you were trying to select"):
with pytest.raises(InvalidIntoExprError, match="\n\nHint: if you were trying"):
nw.to_native(df.select(0)) # type: ignore[arg-type]


@pytest.mark.parametrize("invalid_select", [None, 0, Foo()])
def test_invalid_select(
constructor: Constructor, invalid_select: Any, request: pytest.FixtureRequest
) -> None:
if "polars" in str(constructor) and not isinstance(invalid_select, Foo):
# https://github.com/narwhals-dev/narwhals/issues/1390
request.applymarker(pytest.mark.xfail)
with pytest.raises(InvalidIntoExprError):
nw.from_native(constructor({"a": [1, 2, 3]})).select(invalid_select)


def test_select_boolean_cols(request: pytest.FixtureRequest) -> None:
if PANDAS_VERSION < (1, 1):
# bug in old pandas
Expand Down

0 comments on commit 19df320

Please sign in to comment.