feat: delegate ReadRowsPage.to_arrow to pandas_gbq.arrow - #17938
feat: delegate ReadRowsPage.to_arrow to pandas_gbq.arrow#17938shuoweil wants to merge 18 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request deprecates direct retrieval of Arrow record batches via google-cloud-bigquery-storage and delegates this functionality to a new arrow submodule in pandas-gbq. The review feedback highlights several critical issues: 1) pyarrow.RecordBatch does not have a from_batches method, so from_arrays should be used instead; 2) importing pyarrow at the top level of pandas_gbq/arrow.py will break imports for users who do not have this optional dependency installed, requiring a conditional import and string type annotations; and 3) reader.read_next_batch() can raise a StopIteration exception if the stream is empty, which needs to be handled gracefully.
| """Arrow integration submodule for pandas-gbq.""" | ||
| from typing import Any, Optional | ||
|
|
||
| import pyarrow as pa |
There was a problem hiding this comment.
Since pyarrow is an optional dependency for pandas-gbq, importing it at the top level of this module will cause import pandas_gbq to fail with an ImportError for any user who does not have pyarrow installed. We should wrap the import in a try-except block and raise a descriptive error only when the function is called. Additionally, when assigning None in the except ImportError block, type: ignore[assignment] is necessary to prevent a mypy error.
try:
import pyarrow as pa
except ImportError:
pa = None # type: ignore[assignment]References
- When handling an optional dependency by assigning
Nonein anexcept ImportErrorblock,type: ignore[assignment]is necessary on the assignment to prevent a mypy error, as usingtyping.TYPE_CHECKINGalone is insufficient.
There was a problem hiding this comment.
Fixed. Wrapped the optional pyarrow import in a try...except ImportError block with pa = None # type: ignore[assignment] in pandas_gbq/arrow.py, and added a runtime check in from_read_rows_response that raises a descriptive ImportError when pyarrow is missing. Added test coverage in test_arrow.py.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…row and add mypy type ignore
…ction_does_not_leak_sockets
Working in progress branch
Delegates
ReadRowsPage.to_arrow()ingoogle-cloud-bigquery-storagetopandas_gbq.arrow.from_read_rows_responsewhenpandas-gbqis installed, decoupling protobuf record batch parsing intopandas-gbq.Changes Made
pandas-gbq(packages/pandas-gbq):pandas_gbq.arrow.from_read_rows_response(message, arrow_schema=None)to parseReadRowsResponseprotobuf messages intopyarrow.RecordBatchinstances via IPC stream reading.arrowsubmodule inpandas_gbq.__all__.packages/pandas-gbq/tests/unit/test_arrow.py.google-cloud-bigquery-storage(packages/google-cloud-bigquery-storage):ReadRowsPage.to_arrow()ingoogle/cloud/bigquery_storage_v1/reader.pyto delegate topandas_gbq.arrow.from_read_rows_response.PendingDeprecationWarninginforming users of directpandas-gbqusage.pandas-gbqis not present.packages/google-cloud-bigquery-storage/tests/unit/test_reader_v1_arrow.py.Fixes #<526614511> 🦕