Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 11, 2023
Merged

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

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8c8e73f
API/BUG: Make to_json index= consistent with orient
dshemetov Mar 23, 2023
32ba1ab
style: lint
dshemetov Mar 23, 2023
64e14c3
style: make mypy happy
dshemetov Mar 27, 2023
636b417
review: simplify
dshemetov Mar 28, 2023
38bea0a
Merge remote-tracking branch 'upstream/main' into ds/to_json
dshemetov Mar 28, 2023
2ef117c
Merge branch 'main' into ds/to_json
dshemetov Mar 29, 2023
89cf3c5
review: clarify and consolidate branches
dshemetov Apr 6, 2023
38b81b2
style: add explainer comment
dshemetov Apr 6, 2023
ec728d2
Merge branch 'main' into ds/to_json
dshemetov May 7, 2023
8aef2dd
doc: change error message in _json
dshemetov May 9, 2023
3d84579
docs: update whatsnew 2.1.0
dshemetov May 9, 2023
d53b9f4
docs: sort whatsnew
dshemetov May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions pandas/core/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2277,7 +2277,7 @@ def to_json(
default_handler: Callable[[Any], JSONSerializable] | None = None,
lines: bool_t = False,
compression: CompressionOptions = "infer",
index: bool_t = True,
index: bool_t | None = None,
indent: int | None = None,
storage_options: StorageOptions = None,
mode: Literal["a", "w"] = "w",
Expand Down Expand Up @@ -2346,10 +2346,11 @@ def to_json(

.. versionchanged:: 1.4.0 Zstandard support.

index : bool, default True
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
Copy link
Member

@WillAyd WillAyd Mar 28, 2023

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@dshemetov dshemetov Mar 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Made some changes, let me know what you think!

The index is only used when 'orient' is 'split', 'index', 'column',
or 'table'. Of these, 'index' and 'column' do not support
`index=False`.

indent : int, optional
Length of whitespace used to indent each record.

Expand Down
21 changes: 16 additions & 5 deletions pandas/io/json/_json.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def to_json(
default_handler: Callable[[Any], JSONSerializable] | None = ...,
lines: bool = ...,
compression: CompressionOptions = ...,
index: bool = ...,
index: bool | None = ...,
indent: int = ...,
storage_options: StorageOptions = ...,
mode: Literal["a", "w"] = ...,
Expand All @@ -122,7 +122,7 @@ def to_json(
default_handler: Callable[[Any], JSONSerializable] | None = ...,
lines: bool = ...,
compression: CompressionOptions = ...,
index: bool = ...,
index: bool | None = ...,
indent: int = ...,
storage_options: StorageOptions = ...,
mode: Literal["a", "w"] = ...,
Expand All @@ -141,14 +141,25 @@ def to_json(
default_handler: Callable[[Any], JSONSerializable] | None = None,
lines: bool = False,
compression: CompressionOptions = "infer",
index: bool = True,
index: bool | None = None,
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"]:
Copy link
Member

@WillAyd WillAyd Apr 5, 2023

Choose a reason for hiding this comment

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

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

index = False
elif index is None:
index = True

if not index and orient in ["index", "columns"]:
raise ValueError(
"'index=False' is only valid when 'orient' is 'split', 'table', "
"'records', or 'values'."
)
elif index and orient in ["records", "values"]:
Copy link
Member

@WillAyd WillAyd Apr 5, 2023

Choose a reason for hiding this comment

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

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

raise ValueError(
"'index=False' is only valid when 'orient' is 'split' or 'table'"
"'index=True' is only valid when 'orient' is 'split', 'table', "
"'index', or 'columns'. Convert index to column for other orients."
)

if lines and orient != "records":
Expand Down
23 changes: 20 additions & 3 deletions pandas/tests/io/json/test_pandas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1476,17 +1476,34 @@ def test_index_false_to_json_table(self, data):

assert result == expected

@pytest.mark.parametrize("orient", ["records", "index", "columns", "values"])
@pytest.mark.parametrize("orient", ["index", "columns"])
def test_index_false_error_to_json(self, orient):
# GH 17394
# GH 17394, 25513
# Testing error message from to_json with index=False

df = DataFrame([[1, 2], [4, 5]], columns=["a", "b"])

msg = "'index=False' is only valid when 'orient' is 'split' or 'table'"
msg = (
"'index=False' is only valid when 'orient' is 'split', "
"'table', 'records', or 'values'"
)
with pytest.raises(ValueError, match=msg):
df.to_json(orient=orient, index=False)

@pytest.mark.parametrize("orient", ["records", "values"])
def test_index_true_error_to_json(self, orient):
# GH 25513
# Testing error message from to_json with index=True

df = DataFrame([[1, 2], [4, 5]], columns=["a", "b"])

msg = (
"'index=True' is only valid when 'orient' is 'split', "
"'table', 'index', or 'columns'"
)
with pytest.raises(ValueError, match=msg):
df.to_json(orient=orient, index=True)

@pytest.mark.parametrize("orient", ["split", "table"])
@pytest.mark.parametrize("index", [True, False])
def test_index_false_from_json_to_json(self, orient, index):
Expand Down