Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
6f7a574
unify ColumnNotFound
EdAbati May 4, 2025
8fe45e6
revert
EdAbati May 4, 2025
45f09e0
Merge branch 'main' into unify-column-not-found-error
EdAbati May 4, 2025
6d58bc2
try except during select
EdAbati May 6, 2025
be834b3
catch correct exception
EdAbati May 6, 2025
19d6e24
coverage
EdAbati May 6, 2025
f0a9821
Merge branch 'main' into unify-column-not-found-error
EdAbati May 6, 2025
c1cabd1
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati May 9, 2025
917d073
separate lazy and eager tests
EdAbati May 9, 2025
5d2972d
coverage
EdAbati May 9, 2025
4080101
cleanup exception
EdAbati May 9, 2025
8442596
use constructor_id
EdAbati May 9, 2025
937a123
what is going on in pyspark connect?
EdAbati May 9, 2025
0544f30
ignore pyspark connect
EdAbati May 9, 2025
f41c037
Merge branch 'main' into unify-column-not-found-error
EdAbati May 11, 2025
bb17703
Merge branch 'main' into unify-column-not-found-error
EdAbati May 11, 2025
df18f34
Merge branch 'main' into unify-column-not-found-error
EdAbati May 12, 2025
b518a5a
added missing column tests
EdAbati May 13, 2025
bb7254f
move to drop test
EdAbati May 13, 2025
823d356
fix msg regex
EdAbati May 13, 2025
5647f52
update with columns
EdAbati May 13, 2025
1cb3204
update filter
EdAbati May 13, 2025
901bdf7
catch more precise error
EdAbati May 13, 2025
9cc6a3e
Merge branch 'main' into unify-column-not-found-error
EdAbati May 13, 2025
5432521
remove redundant test
EdAbati May 13, 2025
f51bb19
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati May 19, 2025
d5f51bb
catch duckdb exception func
EdAbati May 22, 2025
b9b0cbf
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati May 22, 2025
048b130
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati May 22, 2025
c9e3061
fix aggregate
EdAbati May 22, 2025
1b77d21
ignore ibis
EdAbati May 23, 2025
40d9abe
test aggregate in select
EdAbati May 23, 2025
0841c6a
catch_pyspark_column_not_found_exception
EdAbati May 23, 2025
29441ef
change signature
EdAbati May 23, 2025
975ef48
testing if connect has the same error?
EdAbati May 23, 2025
eae724d
coverage happy
EdAbati May 23, 2025
661655b
fix spark connect
EdAbati May 23, 2025
a2b2889
catch pyspark connect at collect
EdAbati May 23, 2025
c858861
fixes
EdAbati May 23, 2025
7601ef1
fix regex
EdAbati May 23, 2025
977738f
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati May 23, 2025
49c4806
coverage happier
EdAbati May 23, 2025
76b209d
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati May 24, 2025
6e23961
Merge branch 'main' into unify-column-not-found-error
EdAbati May 24, 2025
dd7a757
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati May 24, 2025
cce992b
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati May 26, 2025
67baf81
fix drop test
EdAbati May 26, 2025
5d1cc71
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati May 26, 2025
be7240f
restore from_available_column_names
EdAbati May 26, 2025
53021e8
Merge branch 'main' into unify-column-not-found-error
EdAbati May 30, 2025
fef9cb3
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati Jun 12, 2025
03cdad2
Merge branch 'main' into unify-column-not-found-error
EdAbati Jun 15, 2025
7c898e1
Merge remote-tracking branch 'upstream/main' into unify-column-not-fo…
EdAbati Jul 8, 2025
5c7e114
remove constructor_lazy
EdAbati Jul 8, 2025
311ed35
Merge branch 'main' into unify-column-not-found-error
MarcoGorelli Jul 13, 2025
aa20974
remove if for only polars
EdAbati Jul 15, 2025
f994666
Merge branch 'main' into unify-column-not-found-error
EdAbati Jul 15, 2025
592ed50
try removing pyspark[connect] from with_columns_missing_column xfails
EdAbati Jul 15, 2025
2240ac0
fix comment
EdAbati Jul 15, 2025
c942c57
fix pyspark[connect] test
EdAbati Jul 15, 2025
710119d
use maybe_collect in more places
EdAbati Jul 15, 2025
1e1edc1
fix comment
EdAbati Jul 15, 2025
b36c866
make coverage happy
EdAbati Jul 15, 2025
00d9cc4
make coverage really happy?
EdAbati Jul 15, 2025
a8ca283
Merge branch 'main' into unify-column-not-found-error
EdAbati Jul 16, 2025
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
10 changes: 9 additions & 1 deletion narwhals/_duckdb/expr.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from narwhals._duckdb.utils import narwhals_to_native_dtype
from narwhals._duckdb.utils import when
from narwhals._expression_parsing import ExprKind
from narwhals.exceptions import ColumnNotFoundError
from narwhals.utils import Implementation
from narwhals.utils import not_implemented
from narwhals.utils import requires
Expand Down Expand Up @@ -186,7 +187,14 @@ def from_column_names(
context: _FullContext,
) -> Self:
def func(df: DuckDBLazyFrame) -> list[duckdb.Expression]:
return [col(name) for name in evaluate_column_names(df)]
col_names = evaluate_column_names(df)
missing_columns = [c for c in col_names if c not in df.columns]

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.

df.columns comes with overhead unfortunately, I think we should avoid calling it where possible. How much overhead depends on the operation

I was hoping we could do something like we do for Polars. That is to say, when we do select / with_columns, we wrap them in try/except, and in the except block we intercept the error message to give a more useful / unified one

@EdAbati EdAbati May 5, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ah interesting, I was not aware πŸ˜•

What is happening in the background in duckdb that causes this overhead ? Do you have a link to the docs? (Just want to learn more)

Also, is it a specific caveat of duckdb? I don't think we should worry about that in spark-like but I might be wrong

I will update the code tonight anyway (but of course feel free to add commits to this branch if you need it for today's release)

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.

df.columns comes with overhead unfortunately, I think we should avoid calling it where possible. How much overhead depends on the operation

@MarcoGorelli could we add that to (#805) and put more of a focus towards it? πŸ™

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.

I don't think it's documented, but evaluating .columns may sometimes require doing a full scan. Example:

In [48]: df = pl.DataFrame({'a': rng.integers(0, 10_000, 100_000_000), 'b': rng.integers(0, 10_000, 100_000_000)})

In [49]: rel = duckdb.table('df')
100% β–•β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–

In [50]: rel1 = duckdb.sql("""pivot rel on a""")

In [51]: %timeit rel.columns
385 ns Β± 7.62 ns per loop (mean Β± std. dev. of 7 runs, 1,000,000 loops each)

In [52]: %timeit rel1.columns
585 ΞΌs Β± 3.8 ΞΌs per loop (mean Β± std. dev. of 7 runs, 1,000 loops each)

Granted, we don't have pivot in the Narwhals lazy API, but a pivot may appear in the history of the relation which someone passes to nw.from_native, and the output schema of pivot is value-dependent (😩 )

The same consideration should apply to spark-like

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.

How do those timings compare to other operations/metadata lookups on the same tables?

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.

.alias for example is completely non-value-dependent, so that stays fast

In [60]: %timeit rel.alias
342 ns Β± 2.3 ns per loop (mean Β± std. dev. of 7 runs, 1,000,000 loops each)

In [61]: %timeit rel1.alias
393 ns Β± 2.6 ns per loop (mean Β± std. dev. of 7 runs, 1,000,000 loops each)

if missing_columns:
raise ColumnNotFoundError.from_missing_and_available_column_names(
missing_columns=missing_columns,
available_columns=df.columns,
)
return [col(name) for name in col_names]

return cls(
func,
Expand Down
8 changes: 8 additions & 0 deletions narwhals/_spark_like/dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from narwhals._spark_like.utils import import_native_dtypes
from narwhals._spark_like.utils import import_window
from narwhals._spark_like.utils import native_to_narwhals_dtype
from narwhals.exceptions import ColumnNotFoundError
from narwhals.exceptions import InvalidOperationError
from narwhals.typing import CompliantLazyFrame
from narwhals.utils import Implementation
Expand Down Expand Up @@ -257,6 +258,13 @@ def collect(
raise ValueError(msg) # pragma: no cover

def simple_select(self, *column_names: str) -> Self:
df_columns = self.columns
missing_columns = [c for c in column_names if c not in df_columns]
if missing_columns:
raise ColumnNotFoundError.from_missing_and_available_column_names(
missing_columns=missing_columns,
available_columns=df_columns,
)
return self._with_native(self.native.select(*column_names))

def aggregate(
Expand Down
10 changes: 9 additions & 1 deletion narwhals/_spark_like/expr.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from narwhals._spark_like.utils import import_window
from narwhals._spark_like.utils import narwhals_to_native_dtype
from narwhals.dependencies import get_pyspark
from narwhals.exceptions import ColumnNotFoundError
from narwhals.utils import Implementation
from narwhals.utils import not_implemented
from narwhals.utils import parse_version
Expand Down Expand Up @@ -224,7 +225,14 @@ def from_column_names(
context: _FullContext,
) -> Self:
def func(df: SparkLikeLazyFrame) -> list[Column]:
return [df._F.col(col_name) for col_name in evaluate_column_names(df)]
col_names = evaluate_column_names(df)
missing_columns = [c for c in col_names if c not in df.columns]
if missing_columns:
raise ColumnNotFoundError.from_missing_and_available_column_names(
missing_columns=missing_columns,
available_columns=df.columns,
)
return [df._F.col(col_name) for col_name in col_names]

return cls(
func,
Expand Down
6 changes: 1 addition & 5 deletions tests/frame/select_test.py
Comment thread
EdAbati marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,7 @@ def test_comparison_with_list_error_message() -> None:
nw.from_native(pd.Series([[1, 2, 3]]), series_only=True) == [1, 2, 3] # noqa: B015


def test_missing_columns(
constructor: Constructor, request: pytest.FixtureRequest
) -> None:
if ("pyspark" in str(constructor)) or "duckdb" in str(constructor):
request.applymarker(pytest.mark.xfail)
def test_missing_columns(constructor: Constructor) -> None:
data = {"a": [1, 3, 2], "b": [4, 4, 6], "z": [7.0, 8.0, 9.0]}
df = nw.from_native(constructor(data))
selected_columns = ["a", "e", "f"]
Expand Down