HyukjinKwon commented on code in PR #59380:
URL: https://github.com/apache/spark/pull/59380#discussion_r4257588740
##########
python/pyspark/pandas/spark/accessors.py:
##########
@@ -815,8 +817,8 @@ def to_spark_io(
>>> df.spark.to_spark_io(path='%s/to_spark_io/foo.json' % path,
format='json')
"""
- if "options" in options and isinstance(options.get("options"), dict)
and len(options) == 1:
- options = options.get("options") # type: ignore[assignment]
+ if options is None:
Review Comment:
**Non-blocking (P2):** Same issue here, and it is also reachable through
`DataFrame.to_delta` / `to_orc`. They only unwrap `options=` when it is the
sole keyword, so `psdf.to_delta(path, options={'mergeSchema': 'true'},
overwriteSchema='true')` now silently loses `overwriteSchema`, which was
applied before this PR. Likewise `psdf.spark.to_spark_io(path, format='json',
options={}, compression='gzip')` writes uncompressed output. Merging the two
forms (`{**(options or {}), **kwargs}`) fixes both paths.
See **Shared repair plan 1** in the review body.
##########
python/pyspark/pandas/spark/accessors.py:
##########
@@ -744,8 +745,8 @@ def to_table(
>>> df.to_table('%s.my_table' % db, partition_cols='date')
"""
- if "options" in options and isinstance(options.get("options"), dict)
and len(options) == 1:
- options = options.get("options") # type: ignore[assignment]
+ if options is None:
Review Comment:
**Non-blocking (P2):** When a caller passes both an `options` dict and
keyword options, the keywords are now silently dropped.
`psdf.spark.to_table('t', options={'a': 'b'}, compression='snappy')` only sends
`a` to `saveAsTable`, and `options={}` plus keywords sends nothing. Before this
change those keywords did reach the writer. `DataFrame.to_table` forwards
`**options` unchanged, so it hits this too. Merging both forms keeps the typing
fix and keeps every option, with keywords winning on a conflict:
```python
options = {**(options or {}), **kwargs}
```
See **Shared repair plan 1** in the review body.
##########
python/pyspark/pandas/spark/accessors.py:
##########
@@ -684,7 +684,8 @@ def to_table(
mode: str = "overwrite",
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
- **options: "OptionalPrimitiveType",
+ options: Optional[Dict[str, "OptionalPrimitiveType"]] = None,
+ **kwargs: "OptionalPrimitiveType",
Review Comment:
**Nit (P3):** The Parameters sections of `to_table` and `to_spark_io` still
describe only `options` ("Additional options passed directly to Spark." /
`options : dict`). They have no entry for the new `**kwargs` and don't say how
the two forms combine. `DataFrame.to_table` also reuses this docstring with its
own `**options` signature. Could you document `options` as an optional dict and
`**kwargs` as extra options merged into it?
See **Shared repair plan 1** in the review body.
--
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]
