timsaucer commented on code in PR #1786:
URL:
https://github.com/apache/datafusion-python/pull/1786#discussion_r4221312353
##########
python/datafusion/user_defined.py:
##########
@@ -1333,7 +1380,7 @@ def _create_table_udf(
@staticmethod
def _create_table_udf_decorator(
- name: str | None = None,
+ name: str,
*,
with_session: bool = False,
) -> Callable[[Callable[..., Any]], TableFunction]:
Review Comment:
The public `udtf` overloads already required `name: str` on main; only this
private helper had a `None` default, which forwarded `None` into
`TableFunction` and failed at runtime. Requiring it here matches the public
contract and surfaces a clear `TypeError`.
##########
.github/workflows/build.yml:
##########
@@ -81,6 +81,10 @@ jobs:
uv run --no-project ruff check --output-format=github python/
uv run --no-project ruff format --check python/
+ - name: Run ty
+ run: |
+ uv run --no-project ty check --output-format github
Review Comment:
`uv sync --dev` installs the pinned `ty==0.0.84` into `.venv`, and `uv run
--no-project` uses that environment, the same as the ruff and codespell steps.
The CI log shows `+ ty==0.0.84`.
##########
python/datafusion/context.py:
##########
@@ -1292,7 +1292,7 @@ def register_csv(
delimiter=delimiter,
schema_infer_max_records=schema_infer_max_records,
file_extension=file_extension,
- file_compression_type=file_compression_type,
+ file_compression_type=file_compression_type or "",
Review Comment:
No behavior change: `CsvReadOptions.file_compression_type` defaults to `""`,
`to_inner()` already applies `or ""`, and the Rust side does
`unwrap_or_default()`.
##########
python/datafusion/context.py:
##########
@@ -2196,7 +2196,7 @@ def read_csv(
schema_infer_max_records=schema_infer_max_records,
file_extension=file_extension,
table_partition_cols=table_partition_cols,
- file_compression_type=file_compression_type,
+ file_compression_type=file_compression_type or "",
Review Comment:
No behavior change: `CsvReadOptions.file_compression_type` defaults to `""`,
`to_inner()` already applies `or ""`, and the Rust side does
`unwrap_or_default()`.
##########
python/datafusion/functions/spark.py:
##########
@@ -56,14 +56,14 @@ def _filter_raw(filter: Expr | None) -> Any:
return filter.expr if filter is not None else None
-def _coerce_i32(value: Expr | int | None) -> Expr | None:
- """Coerce a native ``int`` to an int32 literal, passing ``Expr``/``None``
through.
+def _coerce_i32(value: Expr | int) -> Expr:
+ """Coerce a native ``int`` to an int32 literal, passing ``Expr`` through.
Several Spark datetime and interval builders require 32-bit integer
inputs, so a bare ``int`` must become an int32 literal rather than the
int64 default that :meth:`Expr.literal` would produce.
"""
Review Comment:
All call sites handle `None` before calling (`... if x is not None else
_ZERO_I32`) or take non-optional parameters. On main, a `None` here would have
failed on `.expr`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
