Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
74 changes: 16 additions & 58 deletions google/cloud/bigquery/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from google.cloud.bigquery import standard_sql
from google.cloud.bigquery._helpers import (
_isinstance_or_raise,
_from_api_repr,
_get_sub_prop,
)
from google.cloud.bigquery.enums import StandardSqlTypeNames
Expand Down Expand Up @@ -501,6 +500,7 @@ def _to_schema_fields(schema):
sequence is not a :class:`~google.cloud.bigquery.schema.SchemaField`
instance or a compatible mapping representation of the field.
"""

for field in schema:
if not isinstance(field, (SchemaField, collections.abc.Mapping)):
raise ValueError(
Expand Down Expand Up @@ -598,61 +598,6 @@ def to_api_repr(self) -> dict:
return answer


class TableSchema:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: with this much removed code that isnt relevant to the change, it would probably make sense to have that as a separate PR (for next time don't need to move it now)

"""Schema of a table

Args:
fields (Optional[list]): Describes the fields in a table.
foreignTypeInfo (Optional[str]): Specifies metadata of the foreign data type
definition in field schema.
"""

def __init__(
self, fields: Optional[list] = None, foreign_type_info: Optional[str] = None
):
self._properties: Dict[str, Any] = {}
self.fields = fields
self.foreign_type_info = foreign_type_info

@property
def fields(self) -> Any:
"""Describes the fields in a table."""

return self._properties.get("fields")

@fields.setter
def fields(self, value: list, dtype: str) -> None:
value = _isinstance_or_raise(value, list, none_allowed=True)
self._properties["fields"] = value

@property
def foreign_type_info(self) -> Any:
"""Optional. Specifies metadata of the foreign data type definition in
field schema (TableFieldSchema.foreign_type_definition)."""

return self._properties.get("foreignTypeInfo")

@foreign_type_info.setter
def foreign_type_info(self, value: str, dtype: str) -> None:
if not isinstance(value, str):
raise ValueError(
f"Pass {value} as a '{repr(dtype)}'." f"Got {type(value)}."
)
self._properties["foreignTypeInfo"] = value

def to_api_repr(self) -> dict:
"""Build an API representation of this object.

Returns:
Dict[str, Any]:
A dictionary in the format used by the BigQuery API.
"""
return copy.deepcopy(self._properties)

def from_api_repr(self, resource):
return _from_api_repr(self, resource)


class ForeignTypeInfo:
"""Metadata about the foreign data type definition such as the system in which the
type is defined.
Expand Down Expand Up @@ -687,8 +632,21 @@ def to_api_repr(self) -> dict:
"""
return copy.deepcopy(self._properties)

def from_api_repr(self, resource):
return _from_api_repr(self, resource)
@classmethod
def from_api_repr(cls, resource):
"""Factory: constructs an instance of the class (cls)
given its API representation.

Args:
resource (Dict[str, Any]):
API representation of the object to be instantiated.

Returns:
An instance of the class initialized with data from 'resource'.
"""
config = cls()
config._properties = copy.deepcopy(resource)
return config


class StorageDescriptor:
Expand Down
20 changes: 17 additions & 3 deletions tests/unit/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from google.cloud.bigquery.standard_sql import StandardSqlStructType
from google.cloud.bigquery.schema import (
PolicyTagList,
# ForeignTypeInfo,
ForeignTypeInfo,
StorageDescriptor,
SerDeInfo,
)
Expand Down Expand Up @@ -1121,8 +1121,6 @@ class TestForeignTypeInfo:

@staticmethod
def _get_target_class():
from google.cloud.bigquery.schema import ForeignTypeInfo

return ForeignTypeInfo

def _make_one(self, *args, **kw):
Expand Down Expand Up @@ -1160,6 +1158,22 @@ def test_to_api_repr(self, type_system, expected):
result = self._make_one(type_system=type_system)
assert result.to_api_repr() == expected

def test_from_api_repr(self):
"""GIVEN an api representation of a ForeignTypeInfo object (i.e. resource)
WHEN converted into a ForeignTypeInfo object using from_api_repr() and
displayed as a dict
THEN it will have the same representation a ForeignTypeInfo object created
directly (via _make_one()) and displayed as a dict.
"""
resource = {"typeSystem": "TYPE_SYSTEM_UNSPECIFIED"}

expected = self._make_one(type_system="TYPE_SYSTEM_UNSPECIFIED")

klass = self._get_target_class()
result = klass.from_api_repr(resource)

assert result.to_api_repr() == expected.to_api_repr()
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar feedback to the other CL, though maybe though now I'm thinking maybe we need to test that from_api_repr has the same response, though since to_api_repr is tested I think this is actually ok.

Your doc comment does describe that the results we are comparing are displayed as a dict, maybe its just the test naming that is throwing it off for me (maybe test_convert_api_repr or something?). Either way do think its fine to leave as is.



class TestStorageDescriptor:
"""Tests for the StorageDescriptor class."""
Expand Down