docs: make landing page quickstart runnable#17687
Conversation
4f5da18 to
84b0fb2
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a query delegation mechanism from google-cloud-bigquery to pandas-gbq for high-performance DataFrame generation when a compatible version of pandas-gbq is installed. It adds deprecation warnings for the legacy path and implements PyArrow-to-DataFrame conversion helpers in pandas-gbq. The review feedback highlights several critical improvements to ensure robustness: resolving string-based pandas dtypes before checking for Arrow compatibility, adding defensive checks to prevent KeyErrors when casting user-specified dtypes or converting geography columns that might be missing from the DataFrame, and adding mypy type ignores for optional dependency fallbacks.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/pandas-gbq/pandas_gbq/core/bigquery_helpers.py (126-196)
The current implementation of dtype validation checks hasattr(dtype, "__from_arrow__") directly on the passed arguments. However, if a user passes a string representation of a dtype (such as "boolean", "Int64", or "string"), which is a very common pattern in pandas, the string object itself does not have the __from_arrow__ attribute, causing a false-positive ValueError to be raised.
To fix this, we should resolve the dtype using pandas.api.types.pandas_dtype before checking for the __from_arrow__ attribute.
def _resolve_extension_dtype(dtype, name):
if dtype is None:
return None
resolved = pandas.api.types.pandas_dtype(dtype)
if not hasattr(resolved, "__from_arrow__"):
raise ValueError(f"{name} is not a supported pandas ExtensionDtype")
return resolved
if bool_dtype is DefaultPandasDTypes.BOOL_DTYPE:
bool_dtype = pandas.BooleanDtype()
else:
bool_dtype = _resolve_extension_dtype(bool_dtype, "bool_dtype")
if int_dtype is DefaultPandasDTypes.INT_DTYPE:
int_dtype = pandas.Int64Dtype()
else:
int_dtype = _resolve_extension_dtype(int_dtype, "int_dtype")
if time_dtype is DefaultPandasDTypes.TIME_DTYPE:
if db_dtypes is None:
raise ValueError("db-dtypes is required to convert to time_dtype")
time_dtype = db_dtypes.TimeDtype()
else:
time_dtype = _resolve_extension_dtype(time_dtype, "time_dtype")
float_dtype = _resolve_extension_dtype(float_dtype, "float_dtype")
string_dtype = _resolve_extension_dtype(string_dtype, "string_dtype")
if date_dtype is not DefaultPandasDTypes.DATE_DTYPE:
date_dtype = _resolve_extension_dtype(date_dtype, "date_dtype")
datetime_dtype = _resolve_extension_dtype(datetime_dtype, "datetime_dtype")
timestamp_dtype = _resolve_extension_dtype(timestamp_dtype, "timestamp_dtype")
import packaging.version
_pandas_version = packaging.version.parse(pandas.__version__)
_pyarrow_version = packaging.version.parse(pyarrow.__version__)
supports_range_pyarrow = (
_pandas_version >= packaging.version.Version("1.5.0")
and _pyarrow_version >= packaging.version.Version("10.0.1")
)
if range_date_dtype is DefaultPandasDTypes.RANGE_DATE_DTYPE:
if supports_range_pyarrow:
range_date_dtype = pandas.ArrowDtype(
pyarrow.struct(
[("start", pyarrow.date32()), ("end", pyarrow.date32())]
)
)
else:
range_date_dtype = None
if range_datetime_dtype is DefaultPandasDTypes.RANGE_DATETIME_DTYPE:
if supports_range_pyarrow:
range_datetime_dtype = pandas.ArrowDtype(
pyarrow.struct(
[
("start", pyarrow.timestamp("us")),
("end", pyarrow.timestamp("us")),
]
)
)
else:
range_datetime_dtype = None
if range_timestamp_dtype is DefaultPandasDTypes.RANGE_TIMESTAMP_DTYPE:
if supports_range_pyarrow:
range_timestamp_dtype = pandas.ArrowDtype(
pyarrow.struct(
[
("start", pyarrow.timestamp("us", tz="UTC")),
("end", pyarrow.timestamp("us", tz="UTC")),
]
)
)
else:
range_timestamp_dtype = Nonepackages/pandas-gbq/pandas_gbq/core/bigquery_helpers.py (240-241)
In the original google-cloud-bigquery implementation, any column names specified in dtypes that are not present in the query results are safely ignored. However, the current implementation loops over dtypes and accesses df[column] directly, which will raise a KeyError if a specified column is missing from the DataFrame. We should add a defensive check to ensure the column exists in the DataFrame before casting it.
for column in dtypes:
if column in df:
df[column] = pandas.Series(df[column], dtype=dtypes[column], copy=False)
packages/pandas-gbq/pandas_gbq/core/bigquery_helpers.py (14)
When handling an optional dependency by assigning None in an except ImportError block, type: ignore[assignment] is necessary on the assignment to prevent a mypy error, as using typing.TYPE_CHECKING alone is insufficient.
db_dtypes = 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.
packages/pandas-gbq/pandas_gbq/core/bigquery_helpers.py (20)
When handling an optional dependency by assigning None in an except ImportError block, type: ignore[assignment] is necessary on the assignment to prevent a mypy error, as using typing.TYPE_CHECKING alone is insufficient.
shapely = 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.
packages/pandas-gbq/pandas_gbq/core/bigquery_helpers.py (243-246)
To prevent potential KeyErrors when converting geography columns, we should defensively check if the field name exists in the DataFrame before accessing and converting it.
if geography_as_object:
for field in schema:
if field.field_type.upper() == "GEOGRAPHY" and field.mode != "REPEATED" and field.name in df:
df[field.name] = df[field.name].dropna().apply(wkt.loads)
This review is not for my change in current branch. |
🤖 I have created a release *beep* *boop* --- ## [2.46.0](bigframes-v2.45.0...bigframes-v2.46.0) (2026-07-16) ### Features * **bigframes:** Support groupby.agg/transform with udf transpiler ([#17613](#17613)) ([cae94f9](cae94f9)) * **bigframes:** support offset-based column access via iloc ([#17367](#17367)) ([4253fab](4253fab)) ### Bug Fixes * **bigframes:** Fix sqlglot backend regressions ([#17655](#17655)) ([91f93bc](91f93bc)) * bump gradio from 6.15.0 to 6.15.1 in /packages/bigframes ([#17712](#17712)) ([a85d59f](a85d59f)) * bump mistune from 3.2.1 to 3.3.0 in /packages/bigframes ([#17694](#17694)) ([e5f7fef](e5f7fef)) * bump soupsieve from 2.7 to 2.8.4 in /packages/bigframes ([#17695](#17695)) ([635da34](635da34)) * bump transformers from 5.3.0 to 5.5.0 in /packages/bigframes ([#17700](#17700)) ([4b049c4](4b049c4)) * emit bracketed inline array syntax for scalar subquery expressions ([#17716](#17716)) ([ce5fd50](ce5fd50)) ### Documentation * make landing page quickstart runnable ([fc423c8](fc423c8)) * make landing page quickstart runnable ([#17687](#17687)) ([fc423c8](fc423c8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
docs: make landing page quickstart runnable
Fixes #<522955836> 🦕