Skip to content

REGR: Regression in to_csv for ea dtype categorical #47347

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 9 commits into from
Jun 16, 2022
Merged

REGR: Regression in to_csv for ea dtype categorical #47347

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
809094e
REGR: Regression in to_csv for ea dtype categorical
phofl Jun 14, 2022
700331f
Add test
phofl Jun 14, 2022
b78a2f3
Convert for windows
phofl Jun 14, 2022
90deb17
Fix test
phofl Jun 14, 2022
505ff57
Add issue to whatsnew
phofl Jun 15, 2022
e1e946d
Add parameter and remove comment
phofl Jun 15, 2022
4fd9966
Change to inclusive
phofl Jun 15, 2022
e946026
Merge branch 'main' of https://github.com/pandas-dev/pandas into 46812
phofl Jun 15, 2022
95696aa
Remove comments
phofl Jun 16, 2022
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
Prev Previous commit
Next Next commit
Add parameter and remove comment
  • Loading branch information
@phofl
phofl committed Jun 15, 2022
commit e1e946d69dbc4470039db0e43d1665f13a273938
17 changes: 13 additions & 4 deletions pandas/tests/frame/methods/test_to_csv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1298,10 +1298,19 @@ def test_to_csv_categorical_and_ea(self):

def test_to_csv_categorical_and_interval(self):
# GH#46297
df = DataFrame(
{"a": [pd.Interval(Timestamp("2020-01-01"), Timestamp("2020-01-02"))]}
)
df["a"] = df["a"].astype("category") # astype("object") does not raise an error
with tm.assert_produces_warning(FutureWarning, check_stacklevel=False):
Copy link
Member

Choose a reason for hiding this comment

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

if we assert_produces_warning we will need to remove this on the backport. (which is fine imo)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah Right, this would raise without right?

then let’s change to inclusive immediately and change on the backport to right, this makes more sense imo

Copy link
Member

Choose a reason for hiding this comment

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

sure

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

df = DataFrame(
{
"a": [
pd.Interval(
Timestamp("2020-01-01"),
Timestamp("2020-01-02"),
closed="both",
)
]
}
)
df["a"] = df["a"].astype("category")
result = df.to_csv()
expected_rows = [",a", '0,"[2020-01-01, 2020-01-02]"']
Copy link
Member

Choose a reason for hiding this comment

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

has the default closed argument changed on main?

on 1.3.5 and 1.4.2, the expected would be [",a", '0,"(2020-01-01, 2020-01-02]"'] so this test would fail on backport?

print(pd.__version__)
i = pd.Interval(pd.Timestamp("2020-01-01"), pd.Timestamp("2020-01-02"))
print(repr(i))
print(i)
1.4.2
Interval('2020-01-01', '2020-01-02', closed='right')
(2020-01-01, 2020-01-02]

1.5.0.dev0+958.gf7be58a477
Interval('2020-01-01', '2020-01-02', inclusive='both')
[2020-01-01, 2020-01-02]

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like it. Inclusive was added which defaults to both, when close is not given. I could not find this in the release notes, so we should either add it or change back to right. Probably would have to deprecate first.

For this pr, I will add closed as a keyword and catch the deprecation warning for main, then we can adjust the test as a follow up

Copy link
Member

Choose a reason for hiding this comment

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

For this pr, I will add closed as a keyword and catch the deprecation warning for main, then we can adjust the test as a follow up

sgtm

Copy link
Member Author

Choose a reason for hiding this comment

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

opened #47365

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed here

expected = tm.convert_rows_list_to_csv_str(expected_rows)
Expand Down