API/BUG: Make `to_json` `index=` arg consistent with `orient` arg #52143

dshemetov · 2023-03-23T21:31:56Z

Updates to_json with less surprising behavior with the index arg:

split and table allow index=True/False (as before)
records and values only allow index=False
index and columns only allow index=True
raise for contradictions in the latter two and update messages
add test
closes DataFrame.to_json silently ignores index parameter for most orients. #25513, closes BUG: DataFrame.to_json ignores index if it is repeated in the DataFrame. #37600
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

- split and table allow index=True/False - records and values only allow index=False - index and columns only allow index=True - raise for contradictions in the latter two - see pandas-dev#25513

dshemetov · 2023-03-27T20:19:07Z

Hey @WillAyd, this is a PR for your request here. Let me know who I can tag for review.

WillAyd · 2023-03-28T19:42:33Z

pandas/core/generic.py

-            Whether to include the index values in the JSON string. Not
-            including the index (``index=False``) is only supported when
-            orient is 'split' or 'table'.
+        index : bool or None, default None


I think it would be easier to just say that the index is only used in split, index, column and table orients. Of those formats, index and column cannot be False.

You are kind of doing this now but I think in a way that is a bit more confusing. If you structure the commentary and code this will I think will help simplify the logic

Made some changes, let me know what you think!

WillAyd · 2023-04-05T19:25:47Z

pandas/io/json/_json.py

+            "'index=False' is only valid when 'orient' is 'split', 'table', "
+            "'records', or 'values'."
+        )
+    elif index and orient in ["records", "values"]:


The orient check is the same here as on L149 - can these not be consolidated?

WillAyd · 2023-04-05T19:26:23Z

pandas/io/json/_json.py

    indent: int = 0,
    storage_options: StorageOptions = None,
    mode: Literal["a", "w"] = "w",
 ) -> str | None:
-    if not index and orient not in ["split", "table"]:
+    if index is None and orient in ["records", "values"]:


Is this branch necessary? Or can you just change L151 to set it to True for appropriate orients?

dshemetov · 2023-04-06T22:13:14Z

Consolidated the checks up front, hopefully this is clearer

github-actions · 2023-05-07T00:05:43Z

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

dshemetov · 2023-05-07T02:08:01Z

Waiting on review, friendly ping @WillAyd

WillAyd

I think looks good. This could use a whatsnew note for 2.1. @mroeschke any thoughts on your end?

pandas/io/json/_json.py

WillAyd · 2023-05-09T22:07:42Z

Looks good. Can you add the 2.1 whatsnew note?

dshemetov · 2023-05-09T22:21:01Z

Took a stab at it, lmk if the whatsnew note looks good (never done one before)

mroeschke · 2023-05-09T23:10:17Z

 sort whatsnew entries alphabetically....................................................................Failed
- hook id: sort-whatsnew-items
- duration: 0.05s
- exit code: 1
- files were modified by this hook
pre-commit hook(s) made changes.
If you are seeing this message in CI, reproduce locally with: `pre-commit run --all-files`.
To run `pre-commit` as part of git workflow, use `pre-commit install`.
All changes made by hooks:
diff --git a/doc/source/whatsnew/v2.1.0.rst b/doc/source/whatsnew/v2.1.0.rst
index 56b7327..e0f417e 100644
--- a/doc/source/whatsnew/v2.1.0.rst
+++ b/doc/source/whatsnew/v2.1.0.rst
@@ -96,9 +96,9 @@ Other enhancements
 - Let :meth:`DataFrame.to_feather` accept a non-default :class:`Index` and non-string column names (:issue:`51787`)
 - Performance improvement in :func:`read_csv` (:issue:`52632`) with ``engine="c"``
 - :meth:`Categorical.from_codes` has gotten a ``validate`` parameter (:issue:`50975`)
+- Improved error messaging when using :meth:`DataFrame.to_json` with incompatible ``index`` and ``orient`` arguments (:issue:`52143`)
 - Performance improvement in :func:`concat` with homogeneous ``np.float64`` or ``np.float32`` dtypes (:issue:`52685`)
 - Performance improvement in :meth:`DataFrame.filter` when ``items`` is given (:issue:`52941`)
-- Improved error messaging when using :meth:`DataFrame.to_json` with incompatible ``index`` and ``orient`` arguments (:issue:`52143`)

dshemetov · 2023-05-10T22:06:24Z

@mroeschke sorted!

mroeschke · 2023-05-11T01:01:58Z

Great! Thanks @dshemetov

…ndas-dev#52143) * API/BUG: Make to_json index= consistent with orient - split and table allow index=True/False - records and values only allow index=False - index and columns only allow index=True - raise for contradictions in the latter two - see pandas-dev#25513 * style: lint * style: make mypy happy * review: simplify * review: clarify and consolidate branches * style: add explainer comment * doc: change error message in _json * docs: update whatsnew 2.1.0 * docs: sort whatsnew

A follow-on patch will regenerate Make-managed files. References: * pandas-dev/pandas#52143 Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>

paddymul · 2024-04-05T17:25:36Z

Hey all, for what it's worth I think I'm running into a segfault in release 2.0.3 that was fixed by this PR. Here is my bug report. If it wasn't this PR, it was something that was fixed between 2.0.3 and 2.1.0

#58160

dshemetov · 2024-04-05T19:37:10Z

This PR only disallowed certain combinations of arguments to to_json(), so I don't think it's the cause of the segfault in your report. I don't have any ideas that can help you off the top of my head, but I'll let you know if I do. Best of luck!

dshemetov added 3 commits March 23, 2023 14:28

API/BUG: Make to_json index= consistent with orient

8c8e73f

- split and table allow index=True/False - records and values only allow index=False - index and columns only allow index=True - raise for contradictions in the latter two - see pandas-dev#25513

style: lint

32ba1ab

style: make mypy happy

64e14c3

mroeschke requested a review from WillAyd March 27, 2023 21:47

WillAyd requested changes Mar 28, 2023

View reviewed changes

dshemetov and others added 3 commits March 28, 2023 16:41

review: simplify

636b417

Merge remote-tracking branch 'upstream/main' into ds/to_json

38bea0a

Merge branch 'main' into ds/to_json

2ef117c

dshemetov requested a review from WillAyd April 3, 2023 18:30

WillAyd reviewed Apr 5, 2023

View reviewed changes

review: clarify and consolidate branches

89cf3c5

dshemetov requested a review from WillAyd April 6, 2023 22:13

style: add explainer comment

38b81b2

github-actions bot added the Stale label May 7, 2023

Merge branch 'main' into ds/to_json

ec728d2

WillAyd reviewed May 9, 2023

View reviewed changes

pandas/io/json/_json.py Show resolved Hide resolved

doc: change error message in _json

8aef2dd

docs: update whatsnew 2.1.0

3d84579

WillAyd approved these changes May 9, 2023

View reviewed changes

docs: sort whatsnew

d53b9f4

mroeschke added this to the 2.1 milestone May 11, 2023

mroeschke approved these changes May 11, 2023

View reviewed changes

mroeschke merged commit 0a18ad7 into pandas-dev:main May 11, 2023

dshemetov deleted the ds/to_json branch May 11, 2023 05:11

exs-avianello mentioned this pull request Aug 31, 2023

ValueError: 'index=True' is only valid when 'orient' is 'split', 'table', 'index', or 'columns' huggingface/datasets#6197

Closed

ThomasProctor mentioned this pull request Apr 3, 2024

BUG: DataFrame.to_json ignores index if it is repeated in the DataFrame. #37600

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API/BUG: Make `to_json` `index=` arg consistent with `orient` arg #52143

API/BUG: Make `to_json` `index=` arg consistent with `orient` arg #52143

dshemetov commented Mar 23, 2023 •

edited

Loading

dshemetov commented Mar 27, 2023

WillAyd Mar 28, 2023

dshemetov Mar 28, 2023 •

edited

Loading

WillAyd Apr 5, 2023

WillAyd Apr 5, 2023

dshemetov commented Apr 6, 2023

github-actions bot commented May 7, 2023

dshemetov commented May 7, 2023

WillAyd left a comment

WillAyd commented May 9, 2023

dshemetov commented May 9, 2023

mroeschke commented May 9, 2023

dshemetov commented May 10, 2023

mroeschke commented May 11, 2023

paddymul commented Apr 5, 2024

dshemetov commented Apr 5, 2024 •

edited

Loading

API/BUG: Make to_json index= arg consistent with orient arg #52143

API/BUG: Make to_json index= arg consistent with orient arg #52143

Conversation

dshemetov commented Mar 23, 2023 • edited Loading

dshemetov commented Mar 27, 2023

WillAyd Mar 28, 2023

Choose a reason for hiding this comment

dshemetov Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

WillAyd Apr 5, 2023

Choose a reason for hiding this comment

WillAyd Apr 5, 2023

Choose a reason for hiding this comment

dshemetov commented Apr 6, 2023

github-actions bot commented May 7, 2023

dshemetov commented May 7, 2023

WillAyd left a comment

Choose a reason for hiding this comment

WillAyd commented May 9, 2023

dshemetov commented May 9, 2023

mroeschke commented May 9, 2023

dshemetov commented May 10, 2023

mroeschke commented May 11, 2023

paddymul commented Apr 5, 2024

dshemetov commented Apr 5, 2024 • edited Loading

API/BUG: Make `to_json` `index=` arg consistent with `orient` arg #52143

API/BUG: Make `to_json` `index=` arg consistent with `orient` arg #52143

dshemetov commented Mar 23, 2023 •

edited

Loading

dshemetov Mar 28, 2023 •

edited

Loading

dshemetov commented Apr 5, 2024 •

edited

Loading