Skip to content

Fix roundtripping with pyarrow schema #54768

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 2 commits into from
Aug 26, 2023
Merged

Fix roundtripping with pyarrow schema #54768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63ce323
Fix roundtripping with pyarrow schema
phofl Aug 26, 2023
21017e7
Skip for lower versions
phofl Aug 26, 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
3 changes: 2 additions & 1 deletion pandas/core/arrays/string_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
missing as libmissing,
)
from pandas._libs.arrays import NDArrayBacked
from pandas._libs.lib import ensure_string_array
from pandas.compat import pa_version_under7p0
from pandas.compat.numpy import function as nv
from pandas.util._decorators import doc
Expand Down Expand Up @@ -224,7 +225,7 @@ def __from_arrow__(
arr = np.array([], dtype=object)
else:
arr = pyarrow.concat_arrays(chunks).to_numpy(zero_copy_only=False)
arr = lib.convert_nans_to_NA(arr)
arr = ensure_string_array(arr, na_value=libmissing.NA)
# Bypass validation inside StringArray constructor, see GH#47781
new_string_array = StringArray.__new__(StringArray)
NDArrayBacked.__init__(
Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/io/test_parquet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
""" test parquet compat """
import datetime
from decimal import Decimal
from io import BytesIO
import os
import pathlib
Expand All @@ -16,6 +17,7 @@
from pandas.compat.pyarrow import (
pa_version_under7p0,
pa_version_under8p0,
pa_version_under11p0,
pa_version_under13p0,
)

Expand Down Expand Up @@ -1125,6 +1127,18 @@ def test_string_inference(self, tmp_path, pa):
)
tm.assert_frame_equal(result, expected)

@pytest.mark.skipif(pa_version_under11p0, reason="not supported before 11.0")
def test_roundtrip_decimal(self, tmp_path, pa):
# GH#54768
import pyarrow as pa

path = tmp_path / "decimal.p"
df = pd.DataFrame({"a": [Decimal("123.00")]}, dtype="string[pyarrow]")
df.to_parquet(path, schema=pa.schema([("a", pa.decimal128(5))]))
result = read_parquet(path)
expected = pd.DataFrame({"a": ["123"]}, dtype="string[python]")
tm.assert_frame_equal(result, expected)
Comment on lines +1135 to +1140
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look correct to me? If you specify that the schema should use decimals, it should not come back as string but I would expect object dtype (with decimal.Decimal objects)?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that's because regardless of specifying schema, the original dtype (string) is still stored in the "pandas_metadata", which is used when reading back and converting to pandas (this is something on the pyarrow side, but to be honest this doesn't feel right to me when the user also specifies a schema ..)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this looks buggy, but probably on the arrow side. That said, better strings instead of all NA and we are only restoring the previous behaviour



class TestParquetFastParquet(Base):
def test_basic(self, fp, df_full):
Expand Down