Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 1 addition & 23 deletions narwhals/stable/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
from narwhals.translate import _from_native_impl
from narwhals.translate import get_native_namespace as nw_get_native_namespace
from narwhals.translate import to_native
from narwhals.translate import to_py_scalar as nw_to_py_scalar
from narwhals.translate import to_py_scalar
from narwhals.typing import IntoDataFrameT
from narwhals.typing import IntoFrameT
from narwhals.typing import IntoSeriesT
Expand Down Expand Up @@ -956,28 +956,6 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
return decorator(func)


def to_py_scalar(scalar: Any) -> Any:
"""If a scalar is not Python native, converts it to Python native.

Raises:
ValueError: If the object is not convertible to a scalar.

Examples:
>>> import narwhals.stable.v1 as nw
>>> import pandas as pd
>>> df = nw.from_native(pd.DataFrame({"a": [1, 2, 3]}))
>>> nw.to_py_scalar(df["a"].item(0))
1
>>> import pyarrow as pa
>>> df = nw.from_native(pa.table({"a": [1, 2, 3]}))
>>> nw.to_py_scalar(df["a"].item(0))
1
>>> nw.to_py_scalar(1)
1
"""
return _stableify(nw_to_py_scalar(scalar))


def all() -> Expr:
"""
Instantiate an expression representing all columns.
Expand Down
30 changes: 16 additions & 14 deletions narwhals/translate.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from __future__ import annotations

import numbers
from datetime import datetime
from datetime import timedelta
from functools import wraps
Expand Down Expand Up @@ -46,6 +45,15 @@

T = TypeVar("T")

NON_TEMPORAL_SCALAR_TYPES = (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of related, I am running a test, and getting a raise if None enters this function. Should we add it to non temporal scalar types?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very good point, thanks!

bool,
bytes,
str,
int,
float,
complex,
)


@overload
def to_native(
Expand Down Expand Up @@ -843,7 +851,8 @@ def to_py_scalar(scalar_like: Any) -> Any:
>>> nw.to_py_scalar(1)
1
"""

if isinstance(scalar_like, NON_TEMPORAL_SCALAR_TYPES):
return scalar_like
pa = get_pyarrow()
if pa and isinstance(scalar_like, pa.Scalar):
return scalar_like.as_py()
Expand All @@ -864,18 +873,11 @@ def to_py_scalar(scalar_like: Any) -> Any:
if pd and isinstance(scalar_like, pd.Timedelta):
return scalar_like.to_pytimedelta()

all_scalar_types = (
int,
float,
complex,
bool,
bytes,
str,
datetime,
timedelta,
numbers.Number,
)
if isinstance(scalar_like, all_scalar_types):
# SIM101 suggests merging the tuples, but that would create a new tuple,
# so doesn't seem ideal perf-wise?
if isinstance(scalar_like, NON_TEMPORAL_SCALAR_TYPES) or isinstance( # noqa: SIM101
scalar_like, (datetime, timedelta)
Comment thread
FBruzzesi marked this conversation as resolved.
Outdated
):
return scalar_like

msg = (
Expand Down
4 changes: 4 additions & 0 deletions tests/stable_api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ def test_stable_api_docstrings() -> None:
nw_doc = getattr(nw, item).__doc__
if item == "from_native":
v1_doc = v1_doc.replace("native_dataframe", "native_object")
if item in {"to_py_scalar"}:
# We don't overwrite the docstring for these
Comment on lines +90 to +91

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just so I know what to do/review in the future, which kind of methods should be added here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all of them to be honest, it's probably simpler to just use import narwhals as nw in all docstrings and forget about this test 😅 i'll make a separate pr

assert v1_doc == nw_doc
continue
assert (
v1_doc.replace("import narwhals.stable.v1 as nw", "import narwhals as nw")
== nw_doc
Expand Down