GH-47686: [Docs][Python] Split the Python Parquet docs into separate items#50419
GH-47686: [Docs][Python] Split the Python Parquet docs into separate items#50419AlenkaF wants to merge 17 commits into
Conversation
|
@github-actions crossbow submit preview-docs |
|
Revision: 156850c Submitted crossbow builds: ursacomputing/crossbow @ actions-937dad9db2
|
|
I have used Claude for suggesting the split of pyarrow parquet page and to look through the code and the git log history for any features/changes that might be missing from the docs. From what was suggested I continued with the work on my own. I have split the changes in different commits so the change will be easier to review (due to the docs split, the change is bigger than it really is). commit 1: restructure of the content, no content changes @thisisnic, do you mind having a look? Rendered docs are available here: https://s3.amazonaws.com/arrow-data/pr_docs/50419/python/parquet/index.html |
There was a problem hiding this comment.
Pull request overview
Splits the PyArrow Parquet user-guide documentation into multiple focused pages to improve navigability and enable incremental maintenance, aligning with the broader Python docs revision effort.
Changes:
- Replaces the single large Parquet guide page with a section index plus dedicated subpages (single-file IO, datasets, type handling, encryption).
- Updates the Python docs toctree to point at the new Parquet section entry point.
- Adds/relocates some additional Parquet-related content during the split (e.g., content-defined chunking).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/source/python/parquet/parquet.rst | New “single files” Parquet IO page (read/write, metadata, filtering, etc.). |
| docs/source/python/parquet/parquet_type_handling.rst | New page for Parquet read/write type-handling options. |
| docs/source/python/parquet/parquet_datasets.rst | New page for partitioned datasets / multi-file Parquet usage. |
| docs/source/python/parquet/parquet_encryption.rst | New page for Parquet modular encryption docs. |
| docs/source/python/parquet/index.rst | New Parquet section landing page with toctrees to subpages. |
| docs/source/python/parquet.rst | Removes the previous monolithic Parquet documentation page. |
| docs/source/python/index.rst | Updates the Python guide toctree to reference the new Parquet section entry. |
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you!
I took a read through and this content seems well organized and I appreciate the additions for Geometry/Geography types!
| .. note:: | ||
| Reading GEOMETRY/GEOGRAPHY columns as ``geoarrow.wkb`` additionally | ||
| requires the ``geoarrow.wkb`` extension type to be registered. For that | ||
| you can install Python bindings for GeoArrow | ||
| (`geoarrow-pyarrow <https://geoarrow.org/geoarrow-python/main/index.html#installation>`_) | ||
| and import ``geoarrow.pyarrow`` module. |
| .. code-block:: python | ||
|
|
||
| >>> import numpy as np | ||
| >>> import pandas as pd |
There was a problem hiding this comment.
These examples use Pandas where plain PyArrow could be easily used, which might give the impression to users that they need to use Pandas when reading or writing Parquet. I can see these are existing examples that have been moved though, so this isn't really related to this PR. Maybe as a follow up we should look at using pure PyArrow in more places, except where we are explicitly documenting Pandas integration?
There was a problem hiding this comment.
Yes, I totally agree. We need to make sure pandas is not used for creation of tables only (added that to the docs umbrella issue here).
Why I have kept the pandas df here was due to the content further on connected to maintaining pandas index column, see line 74 and 75.
Co-authored-by: Adam Reeve <adreeve@gmail.com>
|
Thank you all for the reviews! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
docs/source/python/parquet/parquet_type_handling.rst:120
- Sentence casing/grammar: “In addition, We provide …” should be “In addition, we provide …”.
In addition, We provide the ``coerce_timestamps`` option to allow you to select
docs/source/python/parquet/parquet_encryption.rst:71
- Grammar: “as following” should be “as follows”.
defined by :class:`pyarrow.parquet.encryption.KmsClient` as following:
docs/source/python/parquet/parquet_datasets.rst:26
- Grammar: “These may present in a number of ways” is missing “be”.
Multiple Parquet files constitute a Parquet *dataset*. These may present in a
number of ways:
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (4)
docs/source/python/parquet/parquet_encryption.rst:108
:download:target is now one directory deeper (docs/source/python/parquet/…), so../../../python/...resolves underdocs/and breaks the download link. Update the relative path to reach repo root.
An :download:`example <../../../../python/examples/parquet_encryption/sample_vault_kms_client.py>`
docs/source/python/parquet/parquet_type_handling.rst:73
- This code example uses
pq.write_table/pq.read_tablebut doesn't importpyarrow.parquet as pq, making the snippet non-runnable as written.
>>> table = pa.table({'data': pa.array([b'hello', b'world'], pa.binary())})
>>> pq.write_table(table, 'binary.parquet', store_schema=False)
>>> pq.read_table('binary.parquet', binary_type=pa.large_binary()).schema
data: large_binary
docs/source/python/parquet/parquet_type_handling.rst:121
- Sentence casing/grammar: "In addition, We provide" should use lowercase "we".
the desired resolution:
docs/source/python/parquet/parquet_datasets.rst:26
- Grammar: "These may present in a number of ways" is missing a verb (should be "may be present" / "may be presented").
Multiple Parquet files constitute a Parquet *dataset*. These may be present in a
number of ways:
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
docs/source/python/parquet/parquet.rst:284
- The cross-reference uses
:func:`~parquet.read_metadata``` which likely resolves to a top-levelparquetmodule rather thanpyarrow.parquet`` and can break the generated link. Use a relative target under the currentmodule (or the fully-qualified name).
or can also be read directly using :func:`~parquet.read_metadata`:
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
docs/source/python/parquet/parquet.rst:269
:class:~.ParquetFile`` resolves topyarrow.ParquetFileunder `.. currentmodule:: pyarrow`, but the actual class is `pyarrow.parquet.ParquetFile`. Use the fully-qualified reference so Sphinx can link it correctly.
The ``FileMetaData`` of a Parquet file can be accessed through
:class:`~.ParquetFile` as shown above:
docs/source/python/index.rst:57
- Switching the root toctree entry from
parquettoparquet/indexremoves the generatedpython/parquet.htmlpage (the olddocs/source/python/parquet.rstis deleted), which will break existing inbound links/bookmarks. Consider keepingdocs/source/python/parquet.rstas a shim landing page (old URL) that links/includes the new subpages, and keep the toctree entry asparquet.
timestamps
parquet/index
dataset
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
docs/source/python/parquet/parquet.rst:103
- This section labels the output as "RSS" but prints
pa.total_allocated_bytes()(Arrow-allocated bytes), and it also usespathwithout defining it. This makes the example misleading and not directly copy/pasteable.
Because Parquet data needs to be decoded from the Parquet format
and compression, it can't be directly mapped from disk.
Thus the ``memory_map`` option might perform better on some systems
but won't help much with resident memory consumption.
docs/source/python/parquet/index.rst:72
- PR description says the Parquet docs are split into 4 subpages, but the docs index includes an additional subpage (parquet_type_handling). Please update the PR description (or adjust the split) so the change summary matches what is actually being added.
Data Type Handling
------------------
.. toctree::
:maxdepth: 2
parquet_type_handling
|
@github-actions crossbow submit preview-docs |
|
Revision: 58575d9 Submitted crossbow builds: ursacomputing/crossbow @ actions-c31e44a7c8
|
|
@github-actions crossbow submit preview-docs |
|
Revision: 9773c76 Submitted crossbow builds: ursacomputing/crossbow @ actions-4a1d237853
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
docs/source/python/parquet/parquet_encryption.rst:108
- The :download: link path is no longer correct after moving this doc under
docs/source/python/parquet/(it now points underdocs/python/...instead of repo-rootpython/examples/...). This will break the generated download link.
An :download:`example <../../../python/examples/parquet_encryption/sample_vault_kms_client.py>`
docs/source/python/parquet/parquet.rst:112
pa.total_allocated_bytes()reports Arrow memory pool allocations, not OS RSS. Labeling this as "RSS" is misleading (and the sample output repeats that label). Consider renaming to something accurate (or use a real RSS measurement if that’s the intent).
>>> print("RSS: {}MB".format(pa.total_allocated_bytes() >> 20)) # doctest: +SKIP
RSS: 4299MB
>>> pq_array = pq.read_table(path, memory_map=False) # doctest: +SKIP
>>> print("RSS: {}MB".format(pa.total_allocated_bytes() >> 20)) # doctest: +SKIP
docs/source/python/index.rst:56
- Switching the toctree entry from
parquettoparquet/index(and deletingdocs/source/python/parquet.rst) changes the generated page frompython/parquet.htmltopython/parquet/index.html. That will break any existing inbound links/bookmarks to the old URL; consider keeping a thindocs/source/python/parquet.rststub that links/includes the new index page to preserve the old docname.
parquet/index
Rationale for this change
The current parquet section is one-page and covering lots of topics. As per #46601 the parquet section is most visited and would therefore need a bit of maintenance/content update.
What changes are included in this PR?
Are these changes tested?
Via the docs preview
Are there any user-facing changes?
No