Skip to content

Fix: Update type hints for various BigQuery files #2206

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 3 commits into from
May 30, 2025
Merged

Fix: Update type hints for various BigQuery files #2206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e5d7e83
Fix: Update type hints for various BigQuery files
google-labs-jules[bot] May 29, 2025
7f0f639
updates type hints across the board
chalmerlowe May 30, 2025
f4897f3
Merge branch 'main' into fix-type-hints-2132
chalmerlowe May 30, 2025
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
9 changes: 5 additions & 4 deletions google/cloud/bigquery/external_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import base64
import copy
import typing
from typing import Any, Dict, FrozenSet, Iterable, Optional, Union

from google.cloud.bigquery._helpers import _to_bytes
Expand Down Expand Up @@ -835,10 +836,10 @@ def schema(self):
See
https://cloud.google.com/bigquery/docs/reference/rest/v2/tables#ExternalDataConfiguration.FIELDS.schema
"""
# TODO: The typehinting for this needs work. Setting this pragma to temporarily
# manage a pytype issue that came up in another PR. See Issue: #2132
prop = self._properties.get("schema", {}) # type: ignore
return [SchemaField.from_api_repr(field) for field in prop.get("fields", [])] # type: ignore
prop: Dict[str, Any] = typing.cast(
Dict[str, Any], self._properties.get("schema", {})
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be Optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since this method call has a default value:

If you call .get() you will get back the schema dict OR you will get back an empty dict, so I don't think Optional applies here.

)
return [SchemaField.from_api_repr(field) for field in prop.get("fields", [])]

@schema.setter
def schema(self, value):
Expand Down
4 changes: 1 addition & 3 deletions google/cloud/bigquery/job/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,9 +435,7 @@ def __init__(self, job_id, client):
@property
def configuration(self) -> _JobConfig:
"""Job-type specific configurtion."""
# TODO: The typehinting for this needs work. Setting this pragma to temporarily
# manage a pytype issue that came up in another PR. See Issue: #2132
configuration = self._CONFIG_CLASS() # pytype: disable=not-callable
configuration: _JobConfig = self._CONFIG_CLASS() # pytype: disable=not-callable
configuration._properties = self._properties.setdefault("configuration", {})
return configuration

Expand Down
12 changes: 3 additions & 9 deletions google/cloud/bigquery/routine/routine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,23 +518,17 @@ def __init__(self):
@property
def project(self):
"""str: ID of the project containing the routine."""
# TODO: The typehinting for this needs work. Setting this pragma to temporarily
# manage a pytype issue that came up in another PR. See Issue: #2132
return self._properties["projectId"] # pytype: disable=typed-dict-error
return self._properties.get("projectId", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be optional / return none as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All of these (project, dataset_id, routine_id) are used as directly as inputs to a string formatting function. So setting them to None seems slightly more risky than setting them to an empty string. i.e. they should not exist at all (their default state OR be some form of string (empty OR otherwise)).

@property
def path(self):
    """str: URL path for the routine's APIs."""
    return "/projects/%s/datasets/%s/routines/%s" % (
        self.project,
        self.dataset_id,
        self.routine_id,
    )

If we used None, path() could end up producing the following:
"/projects/None/datasets/None/routines/None"
Which might produce undesirable path collisions (not likely, but possible)

Whereas with an empty string we would get:
"/projects//datasets//routines/"
Should result in an error upon receipt by the back end.


@property
def dataset_id(self):
"""str: ID of dataset containing the routine."""
# TODO: The typehinting for this needs work. Setting this pragma to temporarily
# manage a pytype issue that came up in another PR. See Issue: #2132
return self._properties["datasetId"] # pytype: disable=typed-dict-error
return self._properties.get("datasetId", "")

@property
def routine_id(self):
"""str: The routine ID."""
# TODO: The typehinting for this needs work. Setting this pragma to temporarily
# manage a pytype issue that came up in another PR. See Issue: #2132
return self._properties["routineId"] # pytype: disable=typed-dict-error
return self._properties.get("routineId", "")

@property
def path(self):
Expand Down
6 changes: 2 additions & 4 deletions google/cloud/bigquery/schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,9 @@ def __init__(
if max_length is not _DEFAULT_VALUE:
self._properties["maxLength"] = max_length
if policy_tags is not _DEFAULT_VALUE:
# TODO: The typehinting for this needs work. Setting this pragma to temporarily
# manage a pytype issue that came up in another PR. See Issue: #2132
self._properties["policyTags"] = (
policy_tags.to_api_repr() # pytype: disable=attribute-error
if policy_tags is not None
policy_tags.to_api_repr()
if isinstance(policy_tags, PolicyTagList)
else None
)
if isinstance(range_element_type, str):
Expand Down
11 changes: 6 additions & 5 deletions google/cloud/bigquery/table.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ def _reference_getter(table):
return TableReference(dataset_ref, table.table_id)


# TODO: The typehinting for this needs work. Setting this pragma to temporarily
# manage a pytype issue that came up in another PR. See Issue: #2132
def _view_use_legacy_sql_getter(table):
def _view_use_legacy_sql_getter(
table: Union["Table", "TableListItem"]
) -> Optional[bool]:
"""bool: Specifies whether to execute the view with Legacy or Standard SQL.

This boolean specifies whether to execute the view with Legacy SQL
Expand All @@ -151,15 +151,16 @@ def _view_use_legacy_sql_getter(table):
ValueError: For invalid value types.
"""

view = table._properties.get("view") # type: ignore
view: Optional[Dict[str, Any]] = table._properties.get("view")
if view is not None:
# The server-side default for useLegacySql is True.
return view.get("useLegacySql", True) # type: ignore
return view.get("useLegacySql", True) if view is not None else True
# In some cases, such as in a table list no view object is present, but the
# resource still represents a view. Use the type as a fallback.
if table.table_type == "VIEW":
# The server-side default for useLegacySql is True.
return True
return None # explicit return statement to appease mypy


class _TableBase:
Expand Down