Skip to content

BUG: Write compliant Parquet with pyarrow #43690

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

Closed
wants to merge 6 commits into from
Closed

BUG: Write compliant Parquet with pyarrow #43690

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e3f1687
Write compliant Parquet with `pyarrow` if supported
judahrand Sep 21, 2021
39010eb
Import `pa_version_under5p0` in `compat` submodule
judahrand Sep 22, 2021
cc7e6a8
Add whatsnew entry
judahrand Sep 22, 2021
17b4d17
Add test for compliant Parquet
judahrand Sep 22, 2021
5f06acd
Add `check_round_trip` to test
judahrand Sep 22, 2021
4025014
Replace long What's New entry with one liner
judahrand Sep 30, 2021
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
40 changes: 38 additions & 2 deletions doc/source/whatsnew/v1.4.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,45 @@ Now the float-dtype is respected. Since the common dtype for these DataFrames is
res
.. _whatsnew_140.notable_bug_fixes.notable_bug_fix3:
.. _whatsnew_140.notable_bug_fixes.write_compliant_parquet_nested_type:

notable_bug_fix3
Write compliant Parquet nested types if possible
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to add a whole sub-section on this. a single line note is fine.

Copy link
Author

@judahrand judahrand Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I just figured that since it could break people's stuff it was worth calling out but shall remove if you'd prefer?

Let me know.

Copy link
Contributor

@jreback jreback Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

the entire point is this wont' break anyone, yes just a single entry is fine

Copy link
Author

Choose a reason for hiding this comment

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

Do you want to expand on this? This change will change the data that Pandas outputs. If a user is expecting and handling the current output (not necessarily back into Pandas as Parquet is a cross platform format) then this could break their code elsewhere?

Am I misunderstanding your point?

Copy link
Contributor

Choose a reason for hiding this comment

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

just a single line is fine here

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

When using :meth:`DataFrame.to_parquet` to write a DataFrame to Parquet, if any of the columns contained arrays
of values the :mod:`pyarrow` engine would write a non-compliant format. This behavior is now fixed when the installed
version of PyArrow is at least ``4.0.0``.

https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#nested-types

.. ipython:: python
import pandas as pd
import pyarrow.parquet as pq
df = pd.DataFrame({"int_array_col": [[1, 2, 3], [4, 5, 6]]})
df.to_parquet("/tmp/sample_df")
parquet_table = pq.read_table("/tmp/sample_df")
*Previous behavior*:

.. code-block:: ipython
In [4]: parquet_table.schema.types
Out[4]:
[ListType(list<item: int64>)]
*New behavior*:

.. code-block:: ipython
In [4]: parquet_table.schema.types
Out[4]:
[ListType(list<element: int64>)]
.. _whatsnew_140.notable_bug_fixes.notable_bug_fix4:

notable_bug_fix4
^^^^^^^^^^^^^^^^

.. ---------------------------------------------------------------------------
Expand Down
2 changes: 2 additions & 0 deletions pandas/compat/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
pa_version_under2p0,
pa_version_under3p0,
pa_version_under4p0,
pa_version_under5p0,
)

PY39 = sys.version_info >= (3, 9)
Expand Down Expand Up @@ -155,4 +156,5 @@ def get_lzma_file(lzma):
"pa_version_under2p0",
"pa_version_under3p0",
"pa_version_under4p0",
"pa_version_under5p0",
]
7 changes: 7 additions & 0 deletions pandas/io/parquet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
FilePathOrBuffer,
StorageOptions,
)
from pandas.compat import pa_version_under4p0
from pandas.compat._optional import import_optional_dependency
from pandas.errors import AbstractMethodError
from pandas.util._decorators import doc
Expand Down Expand Up @@ -179,6 +180,12 @@ def write(
mode="wb",
is_dir=partition_cols is not None,
)

# Output compliant Parquet if PyArrow supports it and the user hasn't
# explicitly set the desired behavior
if not pa_version_under4p0 and "use_compliant_nested_type" not in kwargs:
kwargs["use_compliant_nested_type"] = True

try:
if partition_cols is not None:
# writes to multiple files under the given path
Expand Down
11 changes: 11 additions & 0 deletions pandas/tests/io/test_parquet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,17 @@ def test_read_parquet_manager(self, pa, using_array_manager):
else:
assert isinstance(result._mgr, pd.core.internals.BlockManager)

@td.skip_if_no("pyarrow", min_version="4.0.0")
def test_list_column_results_in_compliant_parquet(self, pa):
# https://github.com/pandas-dev/pandas/issues/43689
df = pd.DataFrame({"a": [[1], [2]]})

with tm.ensure_clean() as path:
df.to_parquet(path, pa)
result = pyarrow.parquet.read_table(path)

assert str(result.schema.field_by_name("a").type) == "list<element: int64>"
Copy link
Contributor

Choose a reason for hiding this comment

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

use check_round_trip(df, pa) as this will assert the results

Copy link
Author

@judahrand judahrand Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

That isn't what I'm testing though... I'm testing that the Parquet that is written is correct...

ie. "list<element: int64>" not "list<item: int64>"

The serializer and deserializer always worked.

Copy link
Author

Choose a reason for hiding this comment

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

https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#nested-types

Copy link
Author

Choose a reason for hiding this comment

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

apache/arrow#9489

Copy link
Contributor

Choose a reason for hiding this comment

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

well if the serialize & de-serialize works then i am not sure i understand the problem. you can certainly assert the meta-data if you want. the most important point is that you need this option to have correct results yes?

Copy link
Author

@judahrand judahrand Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The problem is that to_parquet doesn't write Parquet compliant data. Parquet is a 'standard' after all. Pandas doesn't follow the standard.

What if I want to load data from Pandas into a different language?

Copy link
Author

@judahrand judahrand Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Here is an example of a place where this is an issue: googleapis/python-bigquery#980
And the use_compliant_nested_type has to be passed manually.

Really use_compliant_nested_type should be the default in PyArrow but it isn't for compatibility reasons. It's all in the Arrow PR I linked.

Copy link
Author

@judahrand judahrand Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

In summary: yes, use_compliant_nested_type is needed in order to output the correct, compliant Parquet format.

But then that comes back around to changing output format and so my point about calling users attention to that in What's New. It they're using the data elsewhere (not Pandas) it could cause them problems.



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