You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/01/23 21:09:28 UTC
[GitHub] [spark] bjornjorgensen opened a new pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
bjornjorgensen opened a new pull request #35296:
URL: https://github.com/apache/spark/pull/35296
### What changes were proposed in this pull request?
This is for SPARK-37981Deletes columns with all Null as default.
Do also see https://github.com/apache/spark/pull/26098
User HyukjinKwon did a reviewed on 21 Oct 2019
"Hey, you should document this in DataFrameWrtier, DataStreamWrtier, readwriter.py"
### Why are the changes needed?
Users need to know why there column(s) with all NaN or Null are gone.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bjornjorgensen commented on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1020043519
>
Added a new post at jira.
Github actions is now enabled.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r790700066
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +904,8 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: If column have only NaN or Null values. The column well be deleted.
Review comment:
We could add a note like you can omit NaN or Null in JSON output by `ignoreNullFields` set to `True` which is a default behaivour of PySpark's JSON writer.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r794155817
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -980,6 +983,9 @@ def to_json(
"""
if "options" in options and isinstance(options.get("options"), dict) and len(options) == 1:
options = options.get("options")
+
+ default_options: Dict[str, Any] = {"ignoreNullFields": False}
+ options = {**default_options, **options}
Review comment:
@bjornjorgensen can you take a look for the test results (https://github.com/bjornjorgensen/spark/runs/4941703748?check_suite_focus=true)? Seems like we should run `dev/reformat-python`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 edited a comment on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019586733
Thank you for your proposal @bjornjorgensen.
First of all, some formalities:
- Could you [enable GitHub actions](https://github.com/apache/spark/pull/35296/checks?check_run_id=4914852660) in your fork,
- It would be better to have JIRA ticket that specifically targets documentation update, as the one you linked doesn't even use `pyspark.pandas`.
Regarding the change ‒ as-is, this doesn't really describe the actual behavior:
- If `path` is not provided, `to_json` delegates transformation to pandas, so fields with NULL are preserved.
- If `path` is provided, standard writer is used, so the same rules apply (in particular `ignoreNullFields` keyword option or `spark.sql.jsonGenerator.ignoreNullFields` config are respected).
- In both cases, behavior doesn't depend on the number of null values in the column.
Finally, you have a typo ‒ "The column **well** be deleted" -> "The column **will** be deleted."
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019586587
Thank your proposal @bjornjorgensen.
First of all, some formalities:
- Could you [enable GitHub actions](https://github.com/apache/spark/pull/35296/checks?check_run_id=4914852660) in your fork,
- It would be better to have JIRA ticket that specifically targets documentation update, as the one you linked doesn't even use `pyspark.pandas`.
Regarding the change ‒ as-is, this doesn't really describe the actual behavior:
- If `path` is not provided, `to_json` delegates transformation to pandas, so fields with NULL are preserved.
- If `path` is provided, standard writer is used, so the same rules apply (in particular `ignoreNullFields` keyword option or `spark.sql.jsonGenerator.ignoreNullFields` config are respected).
- In both cases, behavior doesn't depend on the number of null values in the column.
Finally, you have a typo ‒ "The column **well** be deleted" -> "The column will be deleted."
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 edited a comment on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019586587
Thank you for your proposal @bjornjorgensen.
First of all, some formalities:
- Could you [enable GitHub actions](https://github.com/apache/spark/pull/35296/checks?check_run_id=4914852660) in your fork,
- It would be better to have JIRA ticket that specifically targets documentation update, as the one you linked doesn't even use `pyspark.pandas`.
Regarding the change ‒ as-is, this doesn't really describe the actual behavior:
- If `path` is not provided, `to_json` delegates transformation to pandas, so fields with NULL are preserved.
- If `path` is provided, standard writer is used, so the same rules apply (in particular `ignoreNullFields` keyword option or `spark.sql.jsonGenerator.ignoreNullFields` config are respected).
- In both cases, behavior doesn't depend on the number of null values in the column.
Finally, you have a typo ‒ "The column **well** be deleted" -> "The column will be deleted."
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] itholic edited a comment on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
itholic edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019777437
I think maybe we have something for "Does this PR introduce any user-facing change?" and "How was this patch tested?" section in the PR description.
For example,
**Does this PR introduce any user-facing change?**
Yes, the document is changed.
**How was this patch tested?**
The linter and doc build test should be passed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r790685881
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +904,8 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: If column have only NaN or Null values. The column well be deleted.
Review comment:
So if I understand your correctly, it means either adding a keyword only argument
```python
def to_json(
self,
...,
*,
ignoreNullFields: bool = False,
**options: Any,
) -> Optional[str]:
...
options["ignoreNullFields"] = ignoreNullFields
```
or patching the `options`
```python
def to_json(
self,
...,
**options: Any,
) -> Optional[str]:
...
if "ignoreNullFields" not in options:
options["ignoreNullFields"] = False
```
to ensure that output is consistent independent of `path`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r790698363
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +904,8 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: If column have only NaN or Null values. The column well be deleted.
Review comment:
Let's change koalas code alone for now to match with its default behaviour same as pandas's.
This pandas API on Spark (or legacy Koalas) is pretty new in Spark (it's sort of "alpha component", https://spark.apache.org/versioning-policy.html) vs. other codes in PySpark are pretty old, and many users are dependent on its existing behavior. Also, pandas API on Spark tires to match its behaviour to pandas' whereas Spark does not necessarily follow.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791533824
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -946,6 +947,7 @@ def to_json(
the options in PySpark's API documentation for `spark.write.json(...)`.
It has a higher priority and overwrites all other options.
This parameter only works when `path` is specified.
+ ignoreNullFields: if set to True and path is provided, writer omits columns with all NaN or Null values.
Review comment:
I think it would be even better if we can just convert this to something like:
```
.. note:: Set ignoreNullFields keyword argument to `True` to blah blah ..
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ueshin commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
ueshin commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791057545
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
If we go with the additional parameter, shall we use the keyword-only parameter as @zero323 suggested at https://github.com/apache/spark/pull/35296#discussion_r790685881, but with a snake case parameter name to be more pythonic?
```py
def to_json(
...,
*,
ignore_null_fields: bool = False,
**options) -> ...
```
Then use the original name when merging to `options`.
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,7 +905,7 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
-
+
Review comment:
nit: could you revert this change?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 removed a comment on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 removed a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019586587
Thank you for your proposal @bjornjorgensen.
First of all, some formalities:
- Could you [enable GitHub actions](https://github.com/apache/spark/pull/35296/checks?check_run_id=4914852660) in your fork,
- It would be better to have JIRA ticket that specifically targets documentation update, as the one you linked doesn't even use `pyspark.pandas`.
Regarding the change ‒ as-is, this doesn't really describe the actual behavior:
- If `path` is not provided, `to_json` delegates transformation to pandas, so fields with NULL are preserved.
- If `path` is provided, standard writer is used, so the same rules apply (in particular `ignoreNullFields` keyword option or `spark.sql.jsonGenerator.ignoreNullFields` config are respected).
- In both cases, behavior doesn't depend on the number of null values in the column.
Finally, you have a typo ‒ "The column **well** be deleted" -> "The column will be deleted."
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 edited a comment on pull request #35296: [SPARK-38067][PYTHON] Preserve columns with all None values when saved to JSON file.
Posted by GitBox <gi...@apache.org>.
zero323 edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1025158713
> @zero323 This PR won't have any impact on your first example because there is at least one value that is not None. In your second example it will be without this PR.
It seems like there is still some confusion regarding actual impact of the changes that are proposed here.
So let's start with establishing simple fact ‒ Spark uses *row*-oriented JSON lines format when JSON (that includes `to_json`) is used. `ignoreNullFields` option, by default set to `true` controls writer behavior when missing values are encountered:
- If it is set to `true`, then field is omitted in the output for a *given row*.
```python
>>> import tempfile
>>> path_true_some_null = tempfile.mktemp()
>>> df_some_null = ps.DataFrame.from_dict({"id": [1, 2], "val": [None, 1]})
>>> df_some_null.to_json(path_true_some_null, ignoreNullFields=True)
>>> for x in spark.read.text(path_true_some_null).collect():
... print(x.value)
{"id":2,"val":1.0}
{"id":1}
```
- If it is set to `false`, then field is emitted with JSON `null` (untyped) value.
```python
>>> for x in spark.read.text(path_false_some_null).collect():
... print(x.value)
{"id":1,"val":null}
{"id":2,"val":1.0}
```
Decision is made on row-by-row basis, and is not affected by presence of missing values for the same field in any other row.
Your PR changes default behavior for `pyspark.pandas` JSON writer, from the former (missing values are omitted) to the latter (missing values are preserved). This impacts output, as long as there is any missing value in any field in any row. Furthermore, it can have significant (proportional to N * M, for N rows and M columns) impact in case of sparse datasets (IO cost, storage cost) in case of sparse datasets.
Finally, counting columns in the re-read dataset is simply misleading and your PR doesn't and cannot affect *reader* behavior (it just affects what reader "sees") and, as mentioned before, explicitly writing missing values cannot resolve the problem of properly restoring original schema. Consider the following:
```python
>>> import numpy as np
>>> df_all_null = ps.DataFrame.from_dict({"id": [1, 2], "val": [np.nan, np.nan]})
```
If you write it with `ignoreNullFields` set `true`, it omits the missing values same as in the case where some not missing values are present.
```python
>>> path_true_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_true_all_null, ignoreNullFields=True)
>>> for x in spark.read.text(path_true_all_null).collect():
... print(x.value)
{"id":2}
{"id":1}
```
but there is no trace that other column was ever there.
```python
>>> ps.read_json(path_true_all_null)
id
0 2
```
Setting `ignoreNullFields` set `false`
```python
>>> path_false_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_false_all_null, ignoreNullFields=False)
>>> for x in spark.read.text(path_false_all_null).collect():
... print(x.value)
{"id":1,"val":null}
{"id":2,"val":null}
```
writes JSON `nulls` explicitly (same as in case of (`df_some_null`), and by doing so, provides enough information for the reader to infer that *some* column should be present.
```python
>>> df_all_null_read_with_false = ps.read_json(path_false_all_null)
id val
0 1 None
1 2 None
```
So, does it restore the original schema?
It doesn't. The original contained double values:
```python
>>> df_all_null.to_spark().printSchema()
root
|-- id: long (nullable = false)
|-- val: double (nullable = true)
```
and restored one cannot make any assumptions about the types, so it defaults to strings:
```python
>>> df_all_null_read_with_false.to_spark().printSchema()
root
|-- id: long (nullable = true)
|-- val: string (nullable = true)
```
If user requires specific schema for the frame, schema should be provided on read, which will give the same result independent of specific fields being present in the output or not
```python
>>> ps.read_json(path_false_all_null, schema="id long, val double").to_spark().printSchema()
root
|-- id: long (nullable = true)
|-- val: double (nullable = true)
```
This is standard approach in production pipelines and, on top of consistency, provides significant performance improvements on realistic size data.
So to re-iterate:
- This PR affects the output, as long as dataset contains at least one missing value, and can have significant impact on performance and storage costs.
- Observed impact on reader behavior is more incidental and doesn't guarantee correct schema of the loaded data.
What this PR really achieves, is making output of `to_json(some_path)`
```python
>>> path_pr_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_pr_all_null)
>>> for x in spark.read.text(path_pr_all_null).collect():
... print(x.value)
{"id":2,"val":null}
{"id":1,"val":null}
```
*roughly* equivalent to the output of `to_json()`
```python
>>> df_all_null.to_json()
'[{"id":1,"val":null},{"id":2,"val":null}]'
```
and pandas equivalents.
I wouldn't bother with pointing all of that out, but I have enough experience with users making incorrect assumptions about Spark behavior and nature of fixes, based on misleading JIRA tickets.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bjornjorgensen commented on pull request #35296: [SPARK-38067][PYTHON] Preserve columns with all None values when saved to JSON file.
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1025144269
@zero323 This PR won't have any impact on your first example because there is at least one value that is not None. In your second example it will be without this PR.
`df = ps.DataFrame.from_dict({"id": [1, 2], "val": [np.nan, np.nan]})
df
id val
0 1 NaN
1 2 NaN
df.to_json("testdf.json", num_files=1)
df2 = ps.read_json("testdf.json/*")
df2
id val
0 1 None
1 2 None
df2.to_json("testdf2.json", ignoreNullFields=True, num_files=1)
df3 = ps.read_json("testdf2.json/*")
df3
id
0 1
1 2`
I will change what we agree on, but for now I think this has something to do with None values.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bjornjorgensen commented on pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1021123939
Yes, about testing the only test I find for IO in pandas is for testing csv. I was thinking about making a test where I use shape function to test a file before and after writing and write. But there are another problem pandas prints a tuple with rows, columns pandas_df_json2.shape (6, 4). While pandas on spark print dataframe, columns pandas_api.shape (1, 4).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019657060
Can one of the admins verify this patch?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791266439
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
If we plan to have more Pandas-specific defaults, we could also merge options with defaults dict:
```python
def to_json(
self,
...,
**options: Any,
) -> Optional[str]:
...
default_options: Dict[str, Any] = {"ignoreNullFields": False}
options = {**default_options, **options}
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1021034329
Let's fix up PR title/description too with making it ready for a review. Then I think it's good to go.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r790349225
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +904,8 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: If column have only NaN or Null values. The column well be deleted.
Review comment:
There's `ignoreNullFields` option to control this behaviour. If pandas writes both in JSON, we will have to set this as `False` by default, and document this.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019586630
cc @HyukjinKwon @ueshin FYI
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019589533
Maybe something like this?
> If `path` is provided, writer omits missing values by default. This behavior can be controlled using `ignoreNullFields` option.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1021692805
> Yes, about testing the only test I find for IO in pandas is for testing csv. I was thinking about making a test where I use shape function to test a file before and after writing and write.
One shouldn't really depend on schema inference and reader behavior to test writer for formats which, like JSON lines, provide no schema and / or are less expressive than Spark SQL types. In general case, irrespective of options, the following
```python
spark: SparkSession
df: DataFrame
path: str
df.write.json(path)
assert spark.read.json(path).schema == df.schema
```
is not, and cannot be, guaranteed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1021680222
@ueshin @HyukjinKwon We should create a new ticket for this, that actually matches the problem that is solved, or at least rewrite and reopen one, to which this PR points to. I'll leave decision to you.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bjornjorgensen commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r790691873
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +904,8 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: If column have only NaN or Null values. The column well be deleted.
Review comment:
Yes, this well be the best option. But are you thinking about changing this in https://github.com/apache/spark/pull/26098
// Whether to ignore column of all null during json generating
val structIngoreNull = parameters.getOrElse("structIngoreNull", "true").toBoolean
or just change the “koalas” code?
Yes, pandas writes both in JSON. I have updated the jira post with it.
Or we can add this as a note?
If path is provided, writer omits columns with all NaN or Null values by default. This behavior can be controlled using ignorNullFields option.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791258906
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
maybe let's just don't add it as a parameter for now ..... I prefer second way in https://github.com/apache/spark/pull/35296#discussion_r790685881 because technically `ignoreNullFields` is PySpark options to bypass, and makes less sense to document in pandas api doc surface .. we can just say that these `options` are for PySpark I/O options.
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
Shall we remove this?
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -946,6 +947,7 @@ def to_json(
the options in PySpark's API documentation for `spark.write.json(...)`.
It has a higher priority and overwrites all other options.
This parameter only works when `path` is specified.
+ ignoreNullFields: if set to True and path is provided, writer omits columns with all NaN or Null values.
Review comment:
I think it would be even better if we can just convert this to something like:
```
.. note:: Set ignoreNullFields keyword argument to `True` to blah blah ..
```
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +905,9 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: Set ignoreNullFields keyword argument to `True` and path is provided,
+ writer omits columns with all NaN or Null values.
Review comment:
```suggestion
.. note:: Set `ignoreNullFields` keyword argument to `True` to omit `None` or `NaN` values
when writing JSON objects. It works only when `path` is provided.
```
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
```suggestion
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r790349225
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +904,8 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: If column have only NaN or Null values. The column well be deleted.
Review comment:
There's `ignoreNullFields` option to control this behaviour. If pandas writes both in JSON, we will have to set this as `False` by default, and document this. e.g., set `ignoreNullFields` as `False` if unset.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ueshin commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
ueshin commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791057545
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
If we go with the additional parameter, shall we use the keyword-only parameter as @zero323 suggested at https://github.com/apache/spark/pull/35296#discussion_r790685881, but with a snake case parameter name to be more pythonic?
```py
def to_json(
...,
*,
ignore_null_fields: bool = False,
**options) -> ...
```
Then use the original name when merging to `options`.
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,7 +905,7 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
-
+
Review comment:
nit: could you revert this change?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bjornjorgensen commented on pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1021123939
Yes, about testing the only test I find for IO in pandas is for testing csv. I was thinking about making a test where I use shape function to test a file before and after writing and write. But there are another problem pandas prints a tuple with rows, columns pandas_df_json2.shape (6, 4). While pandas on spark print dataframe, columns pandas_api.shape (1, 4).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on pull request #35296: [SPARK-38067][PYTHON] Preserve `None` values when saved to JSON.
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1026766375
Merged into master.
Thanks everyone!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bjornjorgensen edited a comment on pull request #35296: [SPARK-38067][PYTHON] Preserve columns with all None values when saved to JSON file.
Posted by GitBox <gi...@apache.org>.
bjornjorgensen edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1025144269
@zero323 This PR won't have any impact on your first example because there is at least one value that is not None. In your second example it will be without this PR.
```bash
df = ps.DataFrame.from_dict({"id": [1, 2], "val": [np.nan, np.nan]})
df
id val
0 1 NaN
1 2 NaN
df.to_json("testdf.json", num_files=1)
df2 = ps.read_json("testdf.json/*")
df2
id val
0 1 None
1 2 None
df2.to_json("testdf2.json", ignoreNullFields=True, num_files=1)
df3 = ps.read_json("testdf2.json/*")
df3
id
0 1
1 2
```
I will change what we agree on, but for now I think this has something to do with None values.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bjornjorgensen commented on a change in pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r794889539
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -980,6 +983,9 @@ def to_json(
"""
if "options" in options and isinstance(options.get("options"), dict) and len(options) == 1:
options = options.get("options")
+
+ default_options: Dict[str, Any] = {"ignoreNullFields": False}
+ options = {**default_options, **options}
Review comment:
#dev/reformat-python
reformatted python/pyspark/pandas/generic.py
All done! ✨ 🍰 ✨
1 file reformatted, 367 files left unchanged.
Now flake8 print F821 undefined name 'Dict'
This error is a bug in pyflakes [Why does flake8 give the warning f821 undefined name match when using match](https://stackoverflow.com/questions/67547135/why-does-flake8-give-the-warning-f821-undefined-name-match-when-using-match) We can add # noqa: F821 but then we need to remove this when pyflakes is fixed. I don’t think we should do that.
I have tested this manually. With [bjornjorgensen/spark-notebook:spark-test260122](https://hub.docker.com/layers/bjornjorgensen/spark-notebook/spark-test260122/images/sha256-4738a07b6605b6d84875a2dd604760b67f841ccf2167ec18440df3b11583de3f?context=repo&tab=layers) docker image.
`data = {'col_1': [3, 2, 1, 0], 'col_2': [None, None, None, None]}
test = ps.DataFrame.from_dict(data)
test.to_json("test.json")
test2 = ps.read_json("test.json/*")
test2
col_1 col_2
0 3 None
1 2 None
2 1 None
3 0 None
test2.to_json("test2.json", ignoreNullFields=True)
test3 = ps.read_json("test2.json/*")
test3
col_1
0 3
1 2
2 1
3 0`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r790766474
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +904,8 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: If column have only NaN or Null values. The column well be deleted.
Review comment:
> or just change the “koalas” code?
I concur with @HyukjinKwon ‒ only `pyspark.pandas` code should be changed, and that for the sake of the API consistence.
As of the main writer behavior, I am strongly against changing defaults at all (not only in this PR). In general, omitting NULL fields in the output is the preferred behavior and there is no good justification for flipping the defaults now, after API being stable for years.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791532857
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
Shall we remove this?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ueshin commented on a change in pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
ueshin commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791995794
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -978,6 +981,9 @@ def to_json(
0 a
1 c
"""
+ default_options: Dict[str, Any] = {"ignoreNullFields": False}
+ options = {**default_options, **options}
Review comment:
Merging `default_options` should be after line 988:
```py
if "options" in options and isinstance(options.get("options"), dict) and len(options) == 1:
options = options.get("options")
options = {**default_options, **options}
```
otherwise the if-clause never takes effect.
The if-clause is to help misuse of `options` keyword argument, e.g.,:
```py
psdf.to_json( ..., options={"ignoreNullFields": True})
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ueshin commented on a change in pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
ueshin commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791995794
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -978,6 +981,9 @@ def to_json(
0 a
1 c
"""
+ default_options: Dict[str, Any] = {"ignoreNullFields": False}
+ options = {**default_options, **options}
Review comment:
Merging the default_options should be after line 988.
```py
if "options" in options and isinstance(options.get("options"), dict) and len(options) == 1:
options = options.get("options")
options = {**default_options, **options}
```
otherwise the if-clause never takes effect.
The if-clause is to help misuse of `options` keyword argument, e.g.,:
```py
psdf.to_json( ..., options={"ignoreNullFields": True})
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on a change in pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r795046478
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -980,6 +983,9 @@ def to_json(
"""
if "options" in options and isinstance(options.get("options"), dict) and len(options) == 1:
options = options.get("options")
+
+ default_options: Dict[str, Any] = {"ignoreNullFields": False}
+ options = {**default_options, **options}
Review comment:
> Now flake8 print F821 undefined name 'Dict'
That's because you didn't import `Dict` from `typing` (please note that it is `Dict` not `dict`).
You have to add it after `Callable`.
https://github.com/apache/spark/blob/d992250d93b16ac5301c6b0a0456cad31bddd76b/python/pyspark/pandas/generic.py#L24-L26
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1023821666
Yeah, technically we should create a new JIRA or edit the existing JIRA.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791258906
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
maybe let's just don't add it as a parameter for now ..... I prefer second way in https://github.com/apache/spark/pull/35296#discussion_r790685881 because technically `ignoreNullFields` is PySpark options to bypass, and makes less sense to document in pandas api doc surface .. we can just say that these `options` are for PySpark I/O options.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1021034329
Let's fix up PR title/description too with making it ready for a review. Then I think it's good to go.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791266439
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
If we plan to have more Pandas-specific defaults, we could also merge options with defaults dict:
```python
def to_json(
self,
...,
**options: Any,
) -> Optional[str]:
...
default_options: Dict[str, Any] = {"ignoreNullFields": False}
options = {**default_options, **options}
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 edited a comment on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019586733
Thank you for your proposal @bjornjorgensen.
First of all, some formalities:
- Could you [enable GitHub actions](https://github.com/apache/spark/pull/35296/checks?check_run_id=4914852660) in your fork,
- It would be better to have JIRA ticket that specifically targets documentation update, as the one you linked doesn't even use `pyspark.pandas`.
Regarding the change ‒ as-is, this doesn't really describe the actual behavior:
- If `path` is not provided, `to_json` delegates transformation to pandas, so fields with NULL are preserved.
- If `path` is provided, standard writer is used, so the same rules apply (in particular `ignoreNullFields` keyword option or `spark.sql.jsonGenerator.ignoreNullFields` config are respected).
- In both cases, behavior doesn't depend on the number of null values in the column, as the output is row oriented.
Finally, you have a typo ‒ "The column **well** be deleted" -> "The column **will** be deleted."
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] itholic commented on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
itholic commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019777437
I think maybe we have something for "Does this PR introduce any user-facing change?" and "How was this patch tested?" section in the PR description.
For example,
** Does this PR introduce any user-facing change? **
Yes, the document is changed.
** How was this patch tested? **
The linter and doc build test should be passed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bjornjorgensen commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r790691873
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +904,8 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: If column have only NaN or Null values. The column well be deleted.
Review comment:
Yes, this well be the best option. But are you thinking about changing this in https://github.com/apache/spark/pull/26098
// Whether to ignore column of all null during json generating
val structIngoreNull = parameters.getOrElse("structIngoreNull", "true").toBoolean
or just change the “koalas” code?
Yes, pandas writes both in JSON. I have updated the jira post with it.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 closed pull request #35296: [SPARK-38067][PYTHON] Preserve `None` values when saved to JSON.
Posted by GitBox <gi...@apache.org>.
zero323 closed pull request #35296:
URL: https://github.com/apache/spark/pull/35296
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on pull request #35296: [SPARK-38067][PYTHON] Preserve columns with all None values when saved to JSON file.
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1025158713
> @zero323 This PR won't have any impact on your first example because there is at least one value that is not None. In your second example it will be without this PR.
It seems like there is still some confusion regarding actual impact of the changes that are proposed here.
So let's start with establishing simple fact ‒ Spark uses *row*-oriented JSON lines format when JSON (that includes `to_json`) is used. `ignoreNullFields` option, by default set to `true` controls writer behavior when missing values are encountered:
- If it is set to `true`, then field is omitted in the output for a *given row*.
```python
>>> import tempfile
>>> path_true_some_null = tempfile.mktemp()
>>> df_some_null = ps.DataFrame.from_dict({"id": [1, 2], "val": [None, 1]})
>>> df_some_null.to_json(path_true_some_null, ignoreNullFields=True)
>>> for x in spark.read.text(path_true_some_null).collect():
... print(x.value)
{"id":2,"val":1.0}
{"id":1}
```
- If it is set to `false`, then field is emitted with JSON `null` (untyped) value.
```python
>>> for x in spark.read.text(path_false_some_null).collect():
... print(x.value)
{"id":1,"val":null}
{"id":2,"val":1.0}
```
Decision is made on row-by-row basis, and is not affected by presence of missing values for the same field in any other row.
Your PR changes default behavior for `pyspark.pandas` JSON writer, from the former (missing values are omitted) to the latter (missing values are preserved). This impacts output, as long as there is any missing value in any field in any row. Furthermore, it can have significant (proportional to N * M, for N rows and M columns) impact in case of sparse datasets (IO cost, storage cost) in case of sparse datasets.
Finally, counting columns in the re-read dataset is simply misleading and your PR doesn't and cannot affect *reader* behavior (it just affects what reader "sees") and, as mentioned before, explicitly writing missing values cannot resolve the problem of properly restoring original schema. Consider the following:
```python
>>> import numpy as np
>>> df_all_null = ps.DataFrame.from_dict({"id": [1, 2], "val": [np.nan, np.nan]})
```
If you write it with `ignoreNullFields` set `true`, it omits the missing values same as in the case where some not missing values are present.
```python
>>> path_true_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_true_all_null, ignoreNullFields=True)
>>> for x in spark.read.text(path_true_all_null).collect():
... print(x.value)
{"id":2}
{"id":1}
```
but there is no trace that other column was ever there.
```python
>>> ps.read_json(path_true_all_null)
id
0 2
```
Setting `ignoreNullFields` set `false`
```python
>>> path_false_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_false_all_null, ignoreNullFields=False)
>>> for x in spark.read.text(path_false_all_null).collect():
... print(x.value)
{"id":1,"val":null}
{"id":2,"val":null}
```
writes JSON `nulls` explicitly (same as in case of (`df_some_null`), and by doing so, provides enough information for the reader to infer that *some* column should be present.
```python
>>> df_all_null_read_with_false = ps.read_json(path_false_all_null)
id val
0 1 None
1 2 None
```
So, does it restore the original schema?
It doesn't. The original contained double values:
```python
>>> df_all_null.to_spark().printSchema()
root
|-- id: long (nullable = false)
|-- val: double (nullable = true)
```
and restored one cannot make any assumptions about the types, so it defaults to strings:
```python
>>> df_all_null_read_with_false.to_spark().printSchema()
root
|-- id: long (nullable = true)
|-- val: string (nullable = true)
```
If user requires specific schema for the frame, schema should be provided on read, which will give the same result independent of specific fields being present in the output or not
```python
>>> ps.read_json(path_false_all_null, schema="id long, val double").to_spark().printSchema()
root
|-- id: long (nullable = true)
|-- val: double (nullable = true)
```
This is standard approach in production pipelines and, on top of consistency, provides significant performance improvements on realistic size data.
So to re-iterate:
- This PR affects the output, as long as dataset contains at least one missing value, and can have significant impact on performance and storage costs.
- Observed impact on reader behavior is more incidental and doesn't guarantee correct schema of the loaded data.
What this PR really achieves, is making output of `to_json(some_path)`
```python
>>> path_pr_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_pr_all_null)
>>> for x in spark.read.text(path_pr_all_null).collect():
... print(x.value)
{"id":2,"val":null}
{"id":1,"val":null}
```
roughly equivalent to the output `to_json()`
```python
>>> df_all_null.to_json()
'[{"id":1,"val":null},{"id":2,"val":null}]'
```
and pandas equivalents.
I wouldn't bother with pointing all of that out, but I have enough experience with users making incorrect assumptions about Spark behavior and nature of fixes, based on misleading JIRA tickets.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 edited a comment on pull request #35296: [SPARK-38067][PYTHON] Preserve columns with all None values when saved to JSON file.
Posted by GitBox <gi...@apache.org>.
zero323 edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1025158713
> @zero323 This PR won't have any impact on your first example because there is at least one value that is not None. In your second example it will be without this PR.
It seems like there is still some confusion regarding actual impact of the changes that are proposed here.
So let's start with establishing simple fact ‒ Spark uses *row*-oriented JSON lines format when JSON (that includes `to_json`) is used. `ignoreNullFields` option, by default set to `true` controls writer behavior when missing values are encountered:
- If it is set to `true`, then field is omitted in the output for a *given row*.
```python
>>> import tempfile
>>> path_true_some_null = tempfile.mktemp()
>>> df_some_null = ps.DataFrame.from_dict({"id": [1, 2], "val": [None, 1]})
>>> df_some_null.to_json(path_true_some_null, ignoreNullFields=True)
>>> for x in spark.read.text(path_true_some_null).collect():
... print(x.value)
{"id":2,"val":1.0}
{"id":1}
```
- If it is set to `false`, then field is emitted with JSON `null` (untyped) value.
```python
>>> for x in spark.read.text(path_false_some_null).collect():
... print(x.value)
{"id":1,"val":null}
{"id":2,"val":1.0}
```
Decision is made on row-by-row basis, and is not affected by presence of missing values for the same field in any other row.
Your PR changes default behavior for `pyspark.pandas` JSON writer, from the former (missing values are omitted) to the latter (missing values are preserved). This impacts output, as long as there is any missing value in any field in any row. Furthermore, it can have significant (proportional to N * M, for N rows and M columns) impact in case of sparse datasets (IO cost, storage cost) in case of sparse datasets.
Finally, counting columns in the re-read dataset is simply misleading and your PR doesn't and cannot affect *reader* behavior (it just affects what reader "sees") and, as mentioned before, explicitly writing missing values cannot resolve the problem of properly restoring original schema. Consider the following:
```python
>>> import numpy as np
>>> df_all_null = ps.DataFrame.from_dict({"id": [1, 2], "val": [np.nan, np.nan]})
```
If you write it with `ignoreNullFields` set `true`, it omits the missing values same as in the case where some not missing values are present.
```python
>>> path_true_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_true_all_null, ignoreNullFields=True)
>>> for x in spark.read.text(path_true_all_null).collect():
... print(x.value)
{"id":2}
{"id":1}
```
but there is no trace that other column was ever there.
```python
>>> ps.read_json(path_true_all_null)
id
0 2
```
Setting `ignoreNullFields` set `false`
```python
>>> path_false_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_false_all_null, ignoreNullFields=False)
>>> for x in spark.read.text(path_false_all_null).collect():
... print(x.value)
{"id":1,"val":null}
{"id":2,"val":null}
```
writes JSON `nulls` explicitly (same as in case of (`df_some_null`), and by doing so, provides enough information for the reader to infer that *some* column should be present.
```python
>>> df_all_null_read_with_false = ps.read_json(path_false_all_null)
id val
0 1 None
1 2 None
```
So, does it restore the original schema?
It doesn't. The original contained double values:
```python
>>> df_all_null.to_spark().printSchema()
root
|-- id: long (nullable = false)
|-- val: double (nullable = true)
```
and restored one cannot make any assumptions about the types, so it defaults to strings:
```python
>>> df_all_null_read_with_false.to_spark().printSchema()
root
|-- id: long (nullable = true)
|-- val: string (nullable = true)
```
If user requires specific schema for the frame, schema should be provided on read, which will give the same result independent of specific fields being present in the output or not
```python
>>> ps.read_json(path_false_all_null, schema="id long, val double").to_spark().printSchema()
root
|-- id: long (nullable = true)
|-- val: double (nullable = true)
```
This is standard approach in production pipelines and, on top of consistency, provides significant performance improvements on realistic size data.
So to re-iterate:
- This PR affects the output, as long as dataset contains at least one missing value, and can have significant impact on performance and storage costs.
- Observed impact on reader behavior is more incidental and doesn't guarantee correct schema of the loaded data (not to mention recovering of all column names in case of nested structures).
What this PR really achieves, is making output of `to_json(some_path)`
```python
>>> path_pr_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_pr_all_null)
>>> for x in spark.read.text(path_pr_all_null).collect():
... print(x.value)
{"id":2,"val":null}
{"id":1,"val":null}
```
*roughly* equivalent to the output of `to_json()`
```python
>>> df_all_null.to_json()
'[{"id":1,"val":null},{"id":2,"val":null}]'
```
and pandas equivalents.
I wouldn't bother with pointing all of that out, but I have enough experience with users making incorrect assumptions about Spark behavior and nature of fixes, based on misleading JIRA tickets.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bjornjorgensen commented on pull request #35296: [SPARK-38067][PYTHON] Preserve `None` values when saved to JSON.
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1026608061
@zero323 Thank you
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 edited a comment on pull request #35296: [SPARK-38067][PYTHON] Preserve columns with all None values when saved to JSON file.
Posted by GitBox <gi...@apache.org>.
zero323 edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1025158713
> @zero323 This PR won't have any impact on your first example because there is at least one value that is not None. In your second example it will be without this PR.
It seems like there is still some confusion regarding actual impact of the changes that are proposed here.
So let's start with establishing simple fact ‒ Spark uses *row*-oriented JSON lines format when JSON (that includes `to_json`) is used. `ignoreNullFields` option, by default set to `true` controls writer behavior when missing values are encountered:
- If it is set to `true`, then field is omitted in the output for a *given row*.
```python
>>> import tempfile
>>> path_true_some_null = tempfile.mktemp()
>>> df_some_null = ps.DataFrame.from_dict({"id": [1, 2], "val": [None, 1]})
>>> df_some_null.to_json(path_true_some_null, ignoreNullFields=True)
>>> for x in spark.read.text(path_true_some_null).collect():
... print(x.value)
{"id":2,"val":1.0}
{"id":1}
```
- If it is set to `false`, then field is emitted with JSON `null` (untyped) value.
```python
>>> for x in spark.read.text(path_false_some_null).collect():
... print(x.value)
{"id":1,"val":null}
{"id":2,"val":1.0}
```
Decision is made on row-by-row basis, and is not affected by presence of missing values for the same field in any other row.
Your PR changes default behavior for `pyspark.pandas` JSON writer, from the former (missing values are omitted) to the latter (missing values are preserved). This impacts output, as long as there is any missing value in any field in any row. Furthermore, it can have significant (proportional to N * M, for N rows and M columns) impact in case of sparse datasets (IO cost, storage cost) in case of sparse datasets.
Finally, counting columns in the re-read dataset is simply misleading and your PR doesn't and cannot affect *reader* behavior (it just affects what reader "sees") and, as mentioned before, explicitly writing missing values cannot resolve the problem of properly restoring original schema. Consider the following:
```python
>>> import numpy as np
>>> df_all_null = ps.DataFrame.from_dict({"id": [1, 2], "val": [np.nan, np.nan]})
```
If you write it with `ignoreNullFields` set `true`, it omits the missing values same as in the case where some not missing values are present.
```python
>>> path_true_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_true_all_null, ignoreNullFields=True)
>>> for x in spark.read.text(path_true_all_null).collect():
... print(x.value)
{"id":2}
{"id":1}
```
but there is no trace that other column was ever there.
```python
>>> ps.read_json(path_true_all_null)
id
0 2
```
Setting `ignoreNullFields` set `false`
```python
>>> path_false_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_false_all_null, ignoreNullFields=False)
>>> for x in spark.read.text(path_false_all_null).collect():
... print(x.value)
{"id":1,"val":null}
{"id":2,"val":null}
```
writes JSON `nulls` explicitly (same as in case of (`df_some_null`), and by doing so, provides enough information for the reader to infer that *some* column should be present.
```python
>>> df_all_null_read_with_false = ps.read_json(path_false_all_null)
id val
0 1 None
1 2 None
```
So, does it restore the original schema?
It doesn't. The original contained double values:
```python
>>> df_all_null.to_spark().printSchema()
root
|-- id: long (nullable = false)
|-- val: double (nullable = true)
```
and restored one cannot make any assumptions about the types, so it defaults to strings:
```python
>>> df_all_null_read_with_false.to_spark().printSchema()
root
|-- id: long (nullable = true)
|-- val: string (nullable = true)
```
If user requires specific schema for the frame, schema should be provided on read, which will give the same result independent of specific fields being present in the output or not
```python
>>> ps.read_json(path_false_all_null, schema="id long, val double").to_spark().printSchema()
root
|-- id: long (nullable = true)
|-- val: double (nullable = true)
```
This is standard approach in production pipelines and, on top of consistency, provides significant performance improvements on realistic size data.
So to re-iterate:
- This PR affects the output, as long as dataset contains at least one missing value, and can have significant impact on performance and storage costs.
- Observed impact on reader behavior is more incidental and doesn't guarantee correct schema of the loaded data.
What this PR really achieves, is making output of `to_json(some_path)`
```python
>>> path_pr_all_null = tempfile.mktemp()
>>> df_all_null.to_json(path_pr_all_null)
>>> for x in spark.read.text(path_pr_all_null).collect():
... print(x.value)
{"id":2,"val":null}
{"id":1,"val":null}
```
*roughly* equivalent to the output `to_json()`
```python
>>> df_all_null.to_json()
'[{"id":1,"val":null},{"id":2,"val":null}]'
```
and pandas equivalents.
I wouldn't bother with pointing all of that out, but I have enough experience with users making incorrect assumptions about Spark behavior and nature of fixes, based on misleading JIRA tickets.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on a change in pull request #35296: [SPARK-37981][PYTHON] Change the way pandas saves JSON files.
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r795046654
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -980,6 +983,9 @@ def to_json(
"""
if "options" in options and isinstance(options.get("options"), dict) and len(options) == 1:
options = options.get("options")
+
+ default_options: Dict[str, Any] = {"ignoreNullFields": False}
+ options = {**default_options, **options}
Review comment:
```patch
diff --git a/python/pyspark/pandas/generic.py b/python/pyspark/pandas/generic.py
index c459f033f8..63ce25ec5f 100644
--- a/python/pyspark/pandas/generic.py
+++ b/python/pyspark/pandas/generic.py
@@ -24,6 +24,7 @@ from functools import reduce
from typing import (
Any,
Callable,
+ Dict,
Iterable,
IO,
List,
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35296:
URL: https://github.com/apache/spark/pull/35296#discussion_r791566511
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -904,6 +905,9 @@ def to_json(
.. note:: output JSON format is different from pandas'. It always use `orient='records'`
for its output. This behaviour might have to change in the near future.
+
+ .. note:: Set ignoreNullFields keyword argument to `True` and path is provided,
+ writer omits columns with all NaN or Null values.
Review comment:
```suggestion
.. note:: Set `ignoreNullFields` keyword argument to `True` to omit `None` or `NaN` values
when writing JSON objects. It works only when `path` is provided.
```
##########
File path: python/pyspark/pandas/generic.py
##########
@@ -889,6 +889,7 @@ def to_json(
lines: bool = True,
partition_cols: Optional[Union[str, List[str]]] = None,
index_col: Optional[Union[str, List[str]]] = None,
+ ignoreNullFields: bool = False,
Review comment:
```suggestion
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon edited a comment on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1021034329
Let's fix up PR title/description too with making it ready for a review. Then I think it's good to go. It's better to have a test case but I am personally fine since it bypasses an option. We can add a test in a followup too.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 edited a comment on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019586733
Thank you for your proposal @bjornjorgensen.
First of all, some formalities:
- Could you [enable GitHub actions](https://github.com/apache/spark/pull/35296/checks?check_run_id=4914852660) in your fork?
- It would be better to have JIRA ticket that specifically targets documentation update, as the one you linked doesn't even use `pyspark.pandas`.
Regarding the change ‒ as-is, this doesn't really describe the actual behavior:
- If `path` is not provided, `to_json` delegates transformation to pandas, so fields with NULL are preserved.
- If `path` is provided, standard writer is used, so the same rules apply (in particular `ignoreNullFields` keyword option or `spark.sql.jsonGenerator.ignoreNullFields` config are respected).
- In both cases, behavior doesn't depend on the number of null values in the column, as the output is row oriented.
Finally, you have a typo ‒ "The column **well** be deleted" -> "The column **will** be deleted."
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zero323 commented on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019586733
Thank you for your proposal @bjornjorgensen.
First of all, some formalities:
- Could you [enable GitHub actions](https://github.com/apache/spark/pull/35296/checks?check_run_id=4914852660) in your fork,
- It would be better to have JIRA ticket that specifically targets documentation update, as the one you linked doesn't even use `pyspark.pandas`.
Regarding the change ‒ as-is, this doesn't really describe the actual behavior:
- If `path` is not provided, `to_json` delegates transformation to pandas, so fields with NULL are preserved.
- If `path` is provided, standard writer is used, so the same rules apply (in particular `ignoreNullFields` keyword option or `spark.sql.jsonGenerator.ignoreNullFields` config are respected).
- In both cases, behavior doesn't depend on the number of null values in the column.
Finally, you have a typo ‒ "The column **well** be deleted" -> "The column will be deleted."
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] itholic edited a comment on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
itholic edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1019777437
I think maybe we have something for "Does this PR introduce any user-facing change?" and "How was this patch tested?" section in the PR description.
For example,
**Does this PR introduce any user-facing change?**
Yes, the document for `to_json` function is changed.
**How was this patch tested?**
The linter and doc build test should be passed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon edited a comment on pull request #35296: [SPARK-37981][PYTHON] Add note for deleting Null and NaN
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #35296:
URL: https://github.com/apache/spark/pull/35296#issuecomment-1021034329
Let's fix up PR title/description too with making it ready for a review. Then I think it's good to go. It's better to have a test case but I am personally fine since it bypasses an option. We can add a test in a followup too.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org