From 7c015fcb5f15830be57eedb5d38c4889858938b1 Mon Sep 17 00:00:00 2001 From: Argyris Zymnis Date: Wed, 12 Jul 2017 02:59:43 -0700 Subject: [PATCH 1/9] Add a hashcode implementation to SchemaField --- bigquery/google/cloud/bigquery/schema.py | 18 ++++++++++++------ bigquery/tests/unit/test_schema.py | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/bigquery/google/cloud/bigquery/schema.py b/bigquery/google/cloud/bigquery/schema.py index 6d4a437a809f..70375dcad911 100644 --- a/bigquery/google/cloud/bigquery/schema.py +++ b/bigquery/google/cloud/bigquery/schema.py @@ -43,10 +43,16 @@ def __init__(self, name, field_type, mode='NULLABLE', description=None, self.description = description self.fields = fields - def __eq__(self, other): + def __key(self): return ( - self.name == other.name and - self.field_type.lower() == other.field_type.lower() and - self.mode == other.mode and - self.description == other.description and - self.fields == other.fields) + self.name, + self.field_type.lower(), + self.mode, + self.description, + self.fields) + + def __eq__(self, other): + return self.__key() == other.__key() + + def __hash__(self): + return hash(self.__key()) diff --git a/bigquery/tests/unit/test_schema.py b/bigquery/tests/unit/test_schema.py index 8081fcd6f4e0..7217af67010e 100644 --- a/bigquery/tests/unit/test_schema.py +++ b/bigquery/tests/unit/test_schema.py @@ -111,3 +111,21 @@ def test___eq___hit_w_fields(self): field = self._make_one('test', 'RECORD', fields=[sub1, sub2]) other = self._make_one('test', 'RECORD', fields=[sub1, sub2]) self.assertEqual(field, other) + + def test__hash__set_equality(self): + sub1 = self._make_one('sub1', 'STRING') + sub2 = self._make_one('sub2', 'STRING') + field1 = self._make_one('test', 'RECORD', fields=[sub1]) + field2 = self._make_one('test', 'RECORD', fields=[sub2]) + set_one = {field1, field2} + set_two = {field1, field2} + self.assertEqual(set_one, set_two) + + def test__hash__not_quals(self): + sub1 = self._make_one('sub1', 'STRING') + sub2 = self._make_one('sub2', 'STRING') + field1 = self._make_one('test', 'RECORD', fields=[sub1]) + field2 = self._make_one('test', 'RECORD', fields=[sub2]) + set_one = {field1} + set_two = {field2} + self.assertNotEqual(set_one, set_two) From 3b5c7baa279063de56a94de3f4d4433825e62f2b Mon Sep 17 00:00:00 2001 From: Argyris Zymnis Date: Wed, 12 Jul 2017 09:39:03 -0700 Subject: [PATCH 2/9] Rename key method, add docstring, store fields as tuples --- bigquery/google/cloud/bigquery/schema.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/bigquery/google/cloud/bigquery/schema.py b/bigquery/google/cloud/bigquery/schema.py index 70375dcad911..f91d7ac921b6 100644 --- a/bigquery/google/cloud/bigquery/schema.py +++ b/bigquery/google/cloud/bigquery/schema.py @@ -41,9 +41,13 @@ def __init__(self, name, field_type, mode='NULLABLE', description=None, self.field_type = field_type self.mode = mode self.description = description - self.fields = fields + self.fields = None if(fields is None) else tuple(fields) - def __key(self): + def _key(self): + """ + A tuple describing the contents of this :class:`SchemaField`. + Used to compute this instance's hashcode and evaluate equality. + """ return ( self.name, self.field_type.lower(), @@ -52,7 +56,7 @@ def __key(self): self.fields) def __eq__(self, other): - return self.__key() == other.__key() + return self._key() == other._key() def __hash__(self): - return hash(self.__key()) + return hash(self._key()) From 197455320a071d1adc11dae7f200012cd2f8ea88 Mon Sep 17 00:00:00 2001 From: Argyris Zymnis Date: Mon, 17 Jul 2017 06:38:21 -0700 Subject: [PATCH 3/9] Modify default list of subfields to be the empty tuple --- bigquery/google/cloud/bigquery/schema.py | 9 +++-- bigquery/google/cloud/bigquery/table.py | 6 ++-- bigquery/tests/unit/test_job.py | 4 +-- bigquery/tests/unit/test_query.py | 12 +++---- bigquery/tests/unit/test_schema.py | 13 ++++--- bigquery/tests/unit/test_table.py | 45 ++++++++++++++---------- 6 files changed, 52 insertions(+), 37 deletions(-) diff --git a/bigquery/google/cloud/bigquery/schema.py b/bigquery/google/cloud/bigquery/schema.py index f91d7ac921b6..d799f0a52233 100644 --- a/bigquery/google/cloud/bigquery/schema.py +++ b/bigquery/google/cloud/bigquery/schema.py @@ -32,16 +32,16 @@ class SchemaField(object): :type description: str :param description: optional description for the field. - :type fields: list of :class:`SchemaField`, or None + :type fields: tuple of :class:`SchemaField` :param fields: subfields (requires ``field_type`` of 'RECORD'). """ def __init__(self, name, field_type, mode='NULLABLE', description=None, - fields=None): + fields=()): self.name = name self.field_type = field_type self.mode = mode self.description = description - self.fields = None if(fields is None) else tuple(fields) + self.fields = tuple(fields) def _key(self): """ @@ -60,3 +60,6 @@ def __eq__(self, other): def __hash__(self): return hash(self._key()) + + def __repr__(self): + return "SchemaField{}".format(self._key()) diff --git a/bigquery/google/cloud/bigquery/table.py b/bigquery/google/cloud/bigquery/table.py index 37dc1159cc8e..88327157b59b 100644 --- a/bigquery/google/cloud/bigquery/table.py +++ b/bigquery/google/cloud/bigquery/table.py @@ -1079,7 +1079,7 @@ def _parse_schema_resource(info): present in ``info``. """ if 'fields' not in info: - return None + return () schema = [] for r_field in info['fields']: @@ -1090,7 +1090,7 @@ def _parse_schema_resource(info): sub_fields = _parse_schema_resource(r_field) schema.append( SchemaField(name, field_type, mode, description, sub_fields)) - return schema + return tuple(schema) def _build_schema_resource(fields): @@ -1110,7 +1110,7 @@ def _build_schema_resource(fields): if field.description is not None: info['description'] = field.description if field.fields is not None: - info['fields'] = _build_schema_resource(field.fields) + info['fields'] = tuple(_build_schema_resource(field.fields)) infos.append(info) return infos diff --git a/bigquery/tests/unit/test_job.py b/bigquery/tests/unit/test_job.py index 57d96bf8ae15..8c117a436eef 100644 --- a/bigquery/tests/unit/test_job.py +++ b/bigquery/tests/unit/test_job.py @@ -491,8 +491,8 @@ def test_begin_w_alternate_client(self): 'sourceFormat': 'CSV', 'writeDisposition': 'WRITE_TRUNCATE', 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}, ]} } RESOURCE['configuration']['load'] = LOAD_CONFIGURATION diff --git a/bigquery/tests/unit/test_query.py b/bigquery/tests/unit/test_query.py index d7977a4e7d0c..7d06bc1bd625 100644 --- a/bigquery/tests/unit/test_query.py +++ b/bigquery/tests/unit/test_query.py @@ -42,8 +42,8 @@ def _makeResource(self, complete=False): 'errors': [], 'schema': { 'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQURED'}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQURED'}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQURED', 'fields': ()}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQURED', 'fields': ()}, ], }, } @@ -90,7 +90,7 @@ def _verifySchema(self, query, resource): expected.get('description')) self.assertEqual(found.fields, expected.get('fields')) else: - self.assertIsNone(query.schema) + self.assertEqual(query.schema, ()) def _verifyRows(self, query, resource): expected = resource.get('rows') @@ -166,7 +166,7 @@ def test_ctor_defaults(self): self.assertIsNone(query.page_token) self.assertEqual(query.query_parameters, []) self.assertEqual(query.rows, []) - self.assertIsNone(query.schema) + self.assertEqual(query.schema, ()) self.assertIsNone(query.total_rows) self.assertIsNone(query.total_bytes_processed) self.assertEqual(query.udf_resources, []) @@ -408,8 +408,8 @@ def test_schema(self): resource = { 'schema': { 'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQURED'}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQURED'}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQURED', 'fields': ()}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQURED', 'fields': ()}, ], }, } diff --git a/bigquery/tests/unit/test_schema.py b/bigquery/tests/unit/test_schema.py index 7217af67010e..0e58fc628662 100644 --- a/bigquery/tests/unit/test_schema.py +++ b/bigquery/tests/unit/test_schema.py @@ -32,7 +32,7 @@ def test_ctor_defaults(self): self.assertEqual(field.field_type, 'STRING') self.assertEqual(field.mode, 'NULLABLE') self.assertIsNone(field.description) - self.assertIsNone(field.fields) + self.assertEqual(field.fields, ()) def test_ctor_explicit(self): field = self._make_one('test', 'STRING', mode='REQUIRED', @@ -41,7 +41,7 @@ def test_ctor_explicit(self): self.assertEqual(field.field_type, 'STRING') self.assertEqual(field.mode, 'REQUIRED') self.assertEqual(field.description, 'Testing') - self.assertIsNone(field.fields) + self.assertEqual(field.fields, ()) def test_ctor_subfields(self): field = self._make_one( @@ -57,12 +57,12 @@ def test_ctor_subfields(self): self.assertEqual(field.fields[0].field_type, 'STRING') self.assertEqual(field.fields[0].mode, 'NULLABLE') self.assertIsNone(field.fields[0].description) - self.assertIsNone(field.fields[0].fields) + self.assertEqual(field.fields[0].fields, ()) self.assertEqual(field.fields[1].name, 'local_number') self.assertEqual(field.fields[1].field_type, 'STRING') self.assertEqual(field.fields[1].mode, 'NULLABLE') self.assertIsNone(field.fields[1].description) - self.assertIsNone(field.fields[1].fields) + self.assertEqual(field.fields[1].fields, ()) def test___eq___name_mismatch(self): field = self._make_one('test', 'STRING') @@ -129,3 +129,8 @@ def test__hash__not_quals(self): set_one = {field1} set_two = {field2} self.assertNotEqual(set_one, set_two) + + def test__repr(self): + field1 = self._make_one('field1', 'STRING') + expected = "SchemaField('field1', 'string', 'NULLABLE', None, ())" + self.assertEqual(repr(field1), expected) diff --git a/bigquery/tests/unit/test_table.py b/bigquery/tests/unit/test_table.py index b27736fb896e..11ea31b325f2 100644 --- a/bigquery/tests/unit/test_table.py +++ b/bigquery/tests/unit/test_table.py @@ -465,8 +465,8 @@ def test_create_w_bound_client(self): 'datasetId': self.DS_NAME, 'tableId': self.TABLE_NAME}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -500,8 +500,8 @@ def test_create_w_partition_no_expire(self): 'tableId': self.TABLE_NAME}, 'timePartitioning': {'type': 'DAY'}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -535,8 +535,8 @@ def test_create_w_partition_and_expire(self): 'tableId': self.TABLE_NAME}, 'timePartitioning': {'type': 'DAY', 'expirationMs': 100}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -756,8 +756,8 @@ def test_create_w_missing_output_properties(self): 'datasetId': self.DS_NAME, 'tableId': self.TABLE_NAME}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -912,8 +912,8 @@ def test_patch_w_alternate_client(self): 'location': LOCATION, 'expirationTime': _millis(self.EXP_TIME), 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'NULLABLE'}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'NULLABLE', 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -974,8 +974,8 @@ def test_update_w_bound_client(self): 'datasetId': self.DS_NAME, 'tableId': self.TABLE_NAME}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, 'description': DESCRIPTION, 'friendlyName': TITLE, } @@ -1907,11 +1907,13 @@ def test_defaults(self): self.assertEqual(resource[0], {'name': 'full_name', 'type': 'STRING', - 'mode': 'REQUIRED'}) + 'mode': 'REQUIRED', + 'fields': ()}) self.assertEqual(resource[1], {'name': 'age', 'type': 'INTEGER', - 'mode': 'REQUIRED'}) + 'mode': 'REQUIRED', + 'fields': ()}) def test_w_description(self): from google.cloud.bigquery.table import SchemaField @@ -1926,11 +1928,13 @@ def test_w_description(self): {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', + 'fields': (), 'description': DESCRIPTION}) self.assertEqual(resource[1], {'name': 'age', 'type': 'INTEGER', - 'mode': 'REQUIRED'}) + 'mode': 'REQUIRED', + 'fields': ()}) def test_w_subfields(self): from google.cloud.bigquery.table import SchemaField @@ -1945,17 +1949,20 @@ def test_w_subfields(self): self.assertEqual(resource[0], {'name': 'full_name', 'type': 'STRING', - 'mode': 'REQUIRED'}) + 'mode': 'REQUIRED', + 'fields': ()}) self.assertEqual(resource[1], {'name': 'phone', 'type': 'RECORD', 'mode': 'REPEATABLE', - 'fields': [{'name': 'type', + 'fields': ({'name': 'type', 'type': 'STRING', - 'mode': 'REQUIRED'}, + 'mode': 'REQUIRED', + 'fields': ()}, {'name': 'number', 'type': 'STRING', - 'mode': 'REQUIRED'}]}) + 'mode': 'REQUIRED', + 'fields': ()})}) class _Client(object): From b6507028f97864f1c9d80c29364271c36d6fce30 Mon Sep 17 00:00:00 2001 From: Argyris Zymnis Date: Mon, 17 Jul 2017 09:49:50 -0700 Subject: [PATCH 4/9] Fix linting and remove unused if statement --- bigquery/google/cloud/bigquery/table.py | 3 +- bigquery/tests/unit/test_job.py | 11 ++++- bigquery/tests/unit/test_query.py | 21 +++++++-- bigquery/tests/unit/test_table.py | 60 ++++++++++++++++++++----- 4 files changed, 75 insertions(+), 20 deletions(-) diff --git a/bigquery/google/cloud/bigquery/table.py b/bigquery/google/cloud/bigquery/table.py index 88327157b59b..4f0c934f6a51 100644 --- a/bigquery/google/cloud/bigquery/table.py +++ b/bigquery/google/cloud/bigquery/table.py @@ -1109,8 +1109,7 @@ def _build_schema_resource(fields): 'mode': field.mode} if field.description is not None: info['description'] = field.description - if field.fields is not None: - info['fields'] = tuple(_build_schema_resource(field.fields)) + info['fields'] = tuple(_build_schema_resource(field.fields)) infos.append(info) return infos diff --git a/bigquery/tests/unit/test_job.py b/bigquery/tests/unit/test_job.py index 8c117a436eef..9390061959cb 100644 --- a/bigquery/tests/unit/test_job.py +++ b/bigquery/tests/unit/test_job.py @@ -491,8 +491,15 @@ def test_begin_w_alternate_client(self): 'sourceFormat': 'CSV', 'writeDisposition': 'WRITE_TRUNCATE', 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}, + {'name': + 'full_name', + 'type': 'STRING', + 'mode': 'REQUIRED', + 'fields': ()}, + {'name': 'age', + 'type': 'INTEGER', + 'mode': 'REQUIRED', + 'fields': ()}, ]} } RESOURCE['configuration']['load'] = LOAD_CONFIGURATION diff --git a/bigquery/tests/unit/test_query.py b/bigquery/tests/unit/test_query.py index 7d06bc1bd625..5e5c20ace4a4 100644 --- a/bigquery/tests/unit/test_query.py +++ b/bigquery/tests/unit/test_query.py @@ -42,8 +42,14 @@ def _makeResource(self, complete=False): 'errors': [], 'schema': { 'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQURED', 'fields': ()}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQURED', 'fields': ()}, + {'name': 'full_name', + 'type': 'STRING', + 'mode': 'REQURED', + 'fields': ()}, + {'name': 'age', + 'type': 'INTEGER', + 'mode': 'REQURED', + 'fields': ()}, ], }, } @@ -408,8 +414,15 @@ def test_schema(self): resource = { 'schema': { 'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQURED', 'fields': ()}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQURED', 'fields': ()}, + {'name': + 'full_name', + 'type': 'STRING', + 'mode': 'REQURED', + 'fields': ()}, + {'name': 'age', + 'type': 'INTEGER', + 'mode': 'REQURED', + 'fields': ()}, ], }, } diff --git a/bigquery/tests/unit/test_table.py b/bigquery/tests/unit/test_table.py index 11ea31b325f2..df0d2ae74068 100644 --- a/bigquery/tests/unit/test_table.py +++ b/bigquery/tests/unit/test_table.py @@ -465,8 +465,14 @@ def test_create_w_bound_client(self): 'datasetId': self.DS_NAME, 'tableId': self.TABLE_NAME}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, + {'name': 'full_name', + 'type': 'STRING', + 'mode': 'REQUIRED', + 'fields': ()}, + {'name': 'age', + 'type': 'INTEGER', + 'mode': 'REQUIRED', + 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -500,8 +506,14 @@ def test_create_w_partition_no_expire(self): 'tableId': self.TABLE_NAME}, 'timePartitioning': {'type': 'DAY'}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, + {'name': 'full_name', + 'type': 'STRING', + 'mode': 'REQUIRED', + 'fields': ()}, + {'name': 'age', + 'type': 'INTEGER', + 'mode': 'REQUIRED', + 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -535,8 +547,14 @@ def test_create_w_partition_and_expire(self): 'tableId': self.TABLE_NAME}, 'timePartitioning': {'type': 'DAY', 'expirationMs': 100}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, + {'name': 'full_name', + 'type': 'STRING', + 'mode': 'REQUIRED', + 'fields': ()}, + {'name': 'age', + 'type': 'INTEGER', + 'mode': 'REQUIRED', + 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -756,8 +774,14 @@ def test_create_w_missing_output_properties(self): 'datasetId': self.DS_NAME, 'tableId': self.TABLE_NAME}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, + {'name': 'full_name', + 'type': 'STRING', + 'mode': 'REQUIRED', + 'fields': ()}, + {'name': 'age', + 'type': 'INTEGER', + 'mode': 'REQUIRED', + 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -912,8 +936,14 @@ def test_patch_w_alternate_client(self): 'location': LOCATION, 'expirationTime': _millis(self.EXP_TIME), 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'NULLABLE', 'fields': ()}]}, + {'name': 'full_name', + 'type': 'STRING', + 'mode': 'REQUIRED', + 'fields': ()}, + {'name': 'age', + 'type': 'INTEGER', + 'mode': 'NULLABLE', + 'fields': ()}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -974,8 +1004,14 @@ def test_update_w_bound_client(self): 'datasetId': self.DS_NAME, 'tableId': self.TABLE_NAME}, 'schema': {'fields': [ - {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', 'fields': ()}, - {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED', 'fields': ()}]}, + {'name': 'full_name', + 'type': 'STRING', + 'mode': 'REQUIRED', + 'fields': ()}, + {'name': 'age', + 'type': 'INTEGER', + 'mode': 'REQUIRED', + 'fields': ()}]}, 'description': DESCRIPTION, 'friendlyName': TITLE, } From 5a6a4a0dc9f27bec4d8f1892ff215fe32fb8a237 Mon Sep 17 00:00:00 2001 From: Argyris Zymnis Date: Mon, 17 Jul 2017 09:51:49 -0700 Subject: [PATCH 5/9] Fix a typo --- bigquery/tests/unit/test_schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bigquery/tests/unit/test_schema.py b/bigquery/tests/unit/test_schema.py index 0e58fc628662..d45c2d877c9e 100644 --- a/bigquery/tests/unit/test_schema.py +++ b/bigquery/tests/unit/test_schema.py @@ -121,7 +121,7 @@ def test__hash__set_equality(self): set_two = {field1, field2} self.assertEqual(set_one, set_two) - def test__hash__not_quals(self): + def test__hash__not_equals(self): sub1 = self._make_one('sub1', 'STRING') sub2 = self._make_one('sub2', 'STRING') field1 = self._make_one('test', 'RECORD', fields=[sub1]) From 2f728fad69fbab91c5d33c2e9e0556675f5532bf Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 17 Jul 2017 10:08:01 -0700 Subject: [PATCH 6/9] Making SchemaField immutable. --- bigquery/google/cloud/bigquery/schema.py | 78 +++++++++++++++----- bigquery/tests/unit/test_schema.py | 93 ++++++++++++++++-------- 2 files changed, 122 insertions(+), 49 deletions(-) diff --git a/bigquery/google/cloud/bigquery/schema.py b/bigquery/google/cloud/bigquery/schema.py index d799f0a52233..88d0999d01af 100644 --- a/bigquery/google/cloud/bigquery/schema.py +++ b/bigquery/google/cloud/bigquery/schema.py @@ -26,7 +26,7 @@ class SchemaField(object): 'FLOAT', 'BOOLEAN', 'TIMESTAMP' or 'RECORD'). :type mode: str - :param mode: the type of the field (one of 'NULLABLE', 'REQUIRED', + :param mode: the mode of the field (one of 'NULLABLE', 'REQUIRED', or 'REPEATED'). :type description: str @@ -35,31 +35,75 @@ class SchemaField(object): :type fields: tuple of :class:`SchemaField` :param fields: subfields (requires ``field_type`` of 'RECORD'). """ - def __init__(self, name, field_type, mode='NULLABLE', description=None, - fields=()): - self.name = name - self.field_type = field_type - self.mode = mode - self.description = description - self.fields = tuple(fields) + def __init__(self, name, field_type, mode='NULLABLE', + description=None, fields=()): + self._name = name + self._field_type = field_type + self._mode = mode + self._description = description + self._fields = tuple(fields) - def _key(self): + @property + def name(self): + """str: The name of the field.""" + return self._name + + @property + def field_type(self): + """str: The type of the field. + + Will be one of 'STRING', 'INTEGER', 'FLOAT', 'BOOLEAN', + 'TIMESTAMP' or 'RECORD'. + """ + return self._field_type + + @property + def mode(self): + """str: The mode of the field. + + + Will be one of 'NULLABLE', 'REQUIRED', or 'REPEATED'. """ - A tuple describing the contents of this :class:`SchemaField`. + return self._mode + + @property + def description(self): + """Optional[str]: Description for the field.""" + return self._description + + @property + def fields(self): + """tuple: Subfields contained in this field. + + If ``field_type`` is not 'RECORD', this property must be + empty / unset. + """ + return self._fields + + def _key(self): + """A tuple key that unique-ly describes this field. + Used to compute this instance's hashcode and evaluate equality. + + Returns: + tuple: The contents of this :class:`SchemaField`. """ return ( - self.name, - self.field_type.lower(), - self.mode, - self.description, - self.fields) + self._name, + self._field_type.lower(), + self._mode, + self._description, + self._fields, + ) def __eq__(self, other): - return self._key() == other._key() + if isinstance(other, SchemaField): + return self._key() == other._key() + else: + return NotImplemented def __hash__(self): return hash(self._key()) def __repr__(self): - return "SchemaField{}".format(self._key()) + return 'SchemaField{}'.format(self._key()) diff --git a/bigquery/tests/unit/test_schema.py b/bigquery/tests/unit/test_schema.py index d45c2d877c9e..5a4aa65ea185 100644 --- a/bigquery/tests/unit/test_schema.py +++ b/bigquery/tests/unit/test_schema.py @@ -26,43 +26,72 @@ def _get_target_class(): def _make_one(self, *args, **kw): return self._get_target_class()(*args, **kw) - def test_ctor_defaults(self): + def test_constructor_defaults(self): field = self._make_one('test', 'STRING') - self.assertEqual(field.name, 'test') - self.assertEqual(field.field_type, 'STRING') - self.assertEqual(field.mode, 'NULLABLE') - self.assertIsNone(field.description) - self.assertEqual(field.fields, ()) + self.assertEqual(field._name, 'test') + self.assertEqual(field._field_type, 'STRING') + self.assertEqual(field._mode, 'NULLABLE') + self.assertIsNone(field._description) + self.assertEqual(field._fields, ()) - def test_ctor_explicit(self): + def test_constructor_explicit(self): field = self._make_one('test', 'STRING', mode='REQUIRED', description='Testing') - self.assertEqual(field.name, 'test') - self.assertEqual(field.field_type, 'STRING') - self.assertEqual(field.mode, 'REQUIRED') - self.assertEqual(field.description, 'Testing') - self.assertEqual(field.fields, ()) - - def test_ctor_subfields(self): + self.assertEqual(field._name, 'test') + self.assertEqual(field._field_type, 'STRING') + self.assertEqual(field._mode, 'REQUIRED') + self.assertEqual(field._description, 'Testing') + self.assertEqual(field._fields, ()) + + def test_constructor_subfields(self): + sub_field1 = self._make_one('area_code', 'STRING') + sub_field2 = self._make_one('local_number', 'STRING') field = self._make_one( - 'phone_number', 'RECORD', - fields=[self._make_one('area_code', 'STRING'), - self._make_one('local_number', 'STRING')]) - self.assertEqual(field.name, 'phone_number') - self.assertEqual(field.field_type, 'RECORD') - self.assertEqual(field.mode, 'NULLABLE') - self.assertIsNone(field.description) - self.assertEqual(len(field.fields), 2) - self.assertEqual(field.fields[0].name, 'area_code') - self.assertEqual(field.fields[0].field_type, 'STRING') - self.assertEqual(field.fields[0].mode, 'NULLABLE') - self.assertIsNone(field.fields[0].description) - self.assertEqual(field.fields[0].fields, ()) - self.assertEqual(field.fields[1].name, 'local_number') - self.assertEqual(field.fields[1].field_type, 'STRING') - self.assertEqual(field.fields[1].mode, 'NULLABLE') - self.assertIsNone(field.fields[1].description) - self.assertEqual(field.fields[1].fields, ()) + 'phone_number', + 'RECORD', + fields=[sub_field1, sub_field2], + ) + self.assertEqual(field._name, 'phone_number') + self.assertEqual(field._field_type, 'RECORD') + self.assertEqual(field._mode, 'NULLABLE') + self.assertIsNone(field._description) + self.assertEqual(len(field._fields), 2) + self.assertIs(field._fields[0], sub_field1) + self.assertIs(field._fields[1], sub_field2) + + def test_name_property(self): + name = 'lemon-ness' + schema_field = self._make_one(name, 'INTEGER') + self.assertIs(schema_field.name, name) + + def test_field_type_property(self): + field_type = 'BOOLEAN' + schema_field = self._make_one('whether', field_type) + self.assertIs(schema_field.field_type, field_type) + + def test_mode_property(self): + mode = 'REPEATED' + schema_field = self._make_one('again', 'FLOAT', mode=mode) + self.assertIs(schema_field.mode, mode) + + def test_description_property(self): + description = 'It holds some data.' + schema_field = self._make_one( + 'do', 'TIMESTAMP', description=description) + self.assertIs(schema_field.description, description) + + def test_fields_property(self): + sub_field1 = self._make_one('one', 'STRING') + sub_field2 = self._make_one('fish', 'INTEGER') + fields = (sub_field1, sub_field2) + schema_field = self._make_one('boat', 'RECORD', fields=fields) + self.assertIs(schema_field.fields, fields) + + def test___eq___wrong_type(self): + field = self._make_one('test', 'STRING') + other = object() + self.assertNotEqual(field, other) + self.assertIs(field.__eq__(other), NotImplemented) def test___eq___name_mismatch(self): field = self._make_one('test', 'STRING') From 8882df3f3e64b0286c9fac147847bfacd26067a5 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 17 Jul 2017 10:33:19 -0700 Subject: [PATCH 7/9] Reducing the number of "secondary" changes caused by SchemaField change. --- bigquery/google/cloud/bigquery/table.py | 5 +- bigquery/tests/unit/test_job.py | 11 +--- bigquery/tests/unit/test_query.py | 23 ++----- bigquery/tests/unit/test_schema.py | 6 +- bigquery/tests/unit/test_table.py | 81 ++++++------------------- 5 files changed, 32 insertions(+), 94 deletions(-) diff --git a/bigquery/google/cloud/bigquery/table.py b/bigquery/google/cloud/bigquery/table.py index 4f0c934f6a51..2c4064e83e8f 100644 --- a/bigquery/google/cloud/bigquery/table.py +++ b/bigquery/google/cloud/bigquery/table.py @@ -1090,7 +1090,7 @@ def _parse_schema_resource(info): sub_fields = _parse_schema_resource(r_field) schema.append( SchemaField(name, field_type, mode, description, sub_fields)) - return tuple(schema) + return schema def _build_schema_resource(fields): @@ -1109,7 +1109,8 @@ def _build_schema_resource(fields): 'mode': field.mode} if field.description is not None: info['description'] = field.description - info['fields'] = tuple(_build_schema_resource(field.fields)) + if field.fields: + info['fields'] = _build_schema_resource(field.fields) infos.append(info) return infos diff --git a/bigquery/tests/unit/test_job.py b/bigquery/tests/unit/test_job.py index 9390061959cb..57d96bf8ae15 100644 --- a/bigquery/tests/unit/test_job.py +++ b/bigquery/tests/unit/test_job.py @@ -491,15 +491,8 @@ def test_begin_w_alternate_client(self): 'sourceFormat': 'CSV', 'writeDisposition': 'WRITE_TRUNCATE', 'schema': {'fields': [ - {'name': - 'full_name', - 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}, - {'name': 'age', - 'type': 'INTEGER', - 'mode': 'REQUIRED', - 'fields': ()}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}, ]} } RESOURCE['configuration']['load'] = LOAD_CONFIGURATION diff --git a/bigquery/tests/unit/test_query.py b/bigquery/tests/unit/test_query.py index 5e5c20ace4a4..76d5057f6450 100644 --- a/bigquery/tests/unit/test_query.py +++ b/bigquery/tests/unit/test_query.py @@ -42,14 +42,8 @@ def _makeResource(self, complete=False): 'errors': [], 'schema': { 'fields': [ - {'name': 'full_name', - 'type': 'STRING', - 'mode': 'REQURED', - 'fields': ()}, - {'name': 'age', - 'type': 'INTEGER', - 'mode': 'REQURED', - 'fields': ()}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQURED'}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQURED'}, ], }, } @@ -94,7 +88,7 @@ def _verifySchema(self, query, resource): self.assertEqual(found.mode, expected['mode']) self.assertEqual(found.description, expected.get('description')) - self.assertEqual(found.fields, expected.get('fields')) + self.assertEqual(found.fields, expected.get('fields', ())) else: self.assertEqual(query.schema, ()) @@ -414,15 +408,8 @@ def test_schema(self): resource = { 'schema': { 'fields': [ - {'name': - 'full_name', - 'type': 'STRING', - 'mode': 'REQURED', - 'fields': ()}, - {'name': 'age', - 'type': 'INTEGER', - 'mode': 'REQURED', - 'fields': ()}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQURED'}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQURED'}, ], }, } diff --git a/bigquery/tests/unit/test_schema.py b/bigquery/tests/unit/test_schema.py index 5a4aa65ea185..ba8ca2499844 100644 --- a/bigquery/tests/unit/test_schema.py +++ b/bigquery/tests/unit/test_schema.py @@ -141,7 +141,7 @@ def test___eq___hit_w_fields(self): other = self._make_one('test', 'RECORD', fields=[sub1, sub2]) self.assertEqual(field, other) - def test__hash__set_equality(self): + def test___hash__set_equality(self): sub1 = self._make_one('sub1', 'STRING') sub2 = self._make_one('sub2', 'STRING') field1 = self._make_one('test', 'RECORD', fields=[sub1]) @@ -150,7 +150,7 @@ def test__hash__set_equality(self): set_two = {field1, field2} self.assertEqual(set_one, set_two) - def test__hash__not_equals(self): + def test___hash__not_equals(self): sub1 = self._make_one('sub1', 'STRING') sub2 = self._make_one('sub2', 'STRING') field1 = self._make_one('test', 'RECORD', fields=[sub1]) @@ -159,7 +159,7 @@ def test__hash__not_equals(self): set_two = {field2} self.assertNotEqual(set_one, set_two) - def test__repr(self): + def test___repr__(self): field1 = self._make_one('field1', 'STRING') expected = "SchemaField('field1', 'string', 'NULLABLE', None, ())" self.assertEqual(repr(field1), expected) diff --git a/bigquery/tests/unit/test_table.py b/bigquery/tests/unit/test_table.py index df0d2ae74068..b27736fb896e 100644 --- a/bigquery/tests/unit/test_table.py +++ b/bigquery/tests/unit/test_table.py @@ -465,14 +465,8 @@ def test_create_w_bound_client(self): 'datasetId': self.DS_NAME, 'tableId': self.TABLE_NAME}, 'schema': {'fields': [ - {'name': 'full_name', - 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}, - {'name': 'age', - 'type': 'INTEGER', - 'mode': 'REQUIRED', - 'fields': ()}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -506,14 +500,8 @@ def test_create_w_partition_no_expire(self): 'tableId': self.TABLE_NAME}, 'timePartitioning': {'type': 'DAY'}, 'schema': {'fields': [ - {'name': 'full_name', - 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}, - {'name': 'age', - 'type': 'INTEGER', - 'mode': 'REQUIRED', - 'fields': ()}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -547,14 +535,8 @@ def test_create_w_partition_and_expire(self): 'tableId': self.TABLE_NAME}, 'timePartitioning': {'type': 'DAY', 'expirationMs': 100}, 'schema': {'fields': [ - {'name': 'full_name', - 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}, - {'name': 'age', - 'type': 'INTEGER', - 'mode': 'REQUIRED', - 'fields': ()}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -774,14 +756,8 @@ def test_create_w_missing_output_properties(self): 'datasetId': self.DS_NAME, 'tableId': self.TABLE_NAME}, 'schema': {'fields': [ - {'name': 'full_name', - 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}, - {'name': 'age', - 'type': 'INTEGER', - 'mode': 'REQUIRED', - 'fields': ()}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -936,14 +912,8 @@ def test_patch_w_alternate_client(self): 'location': LOCATION, 'expirationTime': _millis(self.EXP_TIME), 'schema': {'fields': [ - {'name': 'full_name', - 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}, - {'name': 'age', - 'type': 'INTEGER', - 'mode': 'NULLABLE', - 'fields': ()}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'NULLABLE'}]}, } self.assertEqual(req['data'], SENT) self._verifyResourceProperties(table, RESOURCE) @@ -1004,14 +974,8 @@ def test_update_w_bound_client(self): 'datasetId': self.DS_NAME, 'tableId': self.TABLE_NAME}, 'schema': {'fields': [ - {'name': 'full_name', - 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}, - {'name': 'age', - 'type': 'INTEGER', - 'mode': 'REQUIRED', - 'fields': ()}]}, + {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED'}, + {'name': 'age', 'type': 'INTEGER', 'mode': 'REQUIRED'}]}, 'description': DESCRIPTION, 'friendlyName': TITLE, } @@ -1943,13 +1907,11 @@ def test_defaults(self): self.assertEqual(resource[0], {'name': 'full_name', 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}) + 'mode': 'REQUIRED'}) self.assertEqual(resource[1], {'name': 'age', 'type': 'INTEGER', - 'mode': 'REQUIRED', - 'fields': ()}) + 'mode': 'REQUIRED'}) def test_w_description(self): from google.cloud.bigquery.table import SchemaField @@ -1964,13 +1926,11 @@ def test_w_description(self): {'name': 'full_name', 'type': 'STRING', 'mode': 'REQUIRED', - 'fields': (), 'description': DESCRIPTION}) self.assertEqual(resource[1], {'name': 'age', 'type': 'INTEGER', - 'mode': 'REQUIRED', - 'fields': ()}) + 'mode': 'REQUIRED'}) def test_w_subfields(self): from google.cloud.bigquery.table import SchemaField @@ -1985,20 +1945,17 @@ def test_w_subfields(self): self.assertEqual(resource[0], {'name': 'full_name', 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}) + 'mode': 'REQUIRED'}) self.assertEqual(resource[1], {'name': 'phone', 'type': 'RECORD', 'mode': 'REPEATABLE', - 'fields': ({'name': 'type', + 'fields': [{'name': 'type', 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()}, + 'mode': 'REQUIRED'}, {'name': 'number', 'type': 'STRING', - 'mode': 'REQUIRED', - 'fields': ()})}) + 'mode': 'REQUIRED'}]}) class _Client(object): From 3a0ad42ed039573a2bd0437431c098b80434afe8 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 17 Jul 2017 10:34:25 -0700 Subject: [PATCH 8/9] Getting rid of spurious empty line in SchemaField.mode docstring. --- bigquery/google/cloud/bigquery/schema.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bigquery/google/cloud/bigquery/schema.py b/bigquery/google/cloud/bigquery/schema.py index 88d0999d01af..fbba3f3321f3 100644 --- a/bigquery/google/cloud/bigquery/schema.py +++ b/bigquery/google/cloud/bigquery/schema.py @@ -61,7 +61,6 @@ def field_type(self): def mode(self): """str: The mode of the field. - Will be one of 'NULLABLE', 'REQUIRED', or 'REPEATED'. """ return self._mode From c4f423487fccdafd219b0327611490a5378b147d Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 17 Jul 2017 10:40:20 -0700 Subject: [PATCH 9/9] Adding SchemaField.__ne__. --- bigquery/google/cloud/bigquery/schema.py | 6 ++++++ bigquery/tests/unit/test_schema.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/bigquery/google/cloud/bigquery/schema.py b/bigquery/google/cloud/bigquery/schema.py index fbba3f3321f3..faec69f616da 100644 --- a/bigquery/google/cloud/bigquery/schema.py +++ b/bigquery/google/cloud/bigquery/schema.py @@ -101,6 +101,12 @@ def __eq__(self, other): else: return NotImplemented + def __ne__(self, other): + if isinstance(other, SchemaField): + return self._key() != other._key() + else: + return NotImplemented + def __hash__(self): return hash(self._key()) diff --git a/bigquery/tests/unit/test_schema.py b/bigquery/tests/unit/test_schema.py index ba8ca2499844..018736d31bc1 100644 --- a/bigquery/tests/unit/test_schema.py +++ b/bigquery/tests/unit/test_schema.py @@ -141,6 +141,26 @@ def test___eq___hit_w_fields(self): other = self._make_one('test', 'RECORD', fields=[sub1, sub2]) self.assertEqual(field, other) + def test___ne___wrong_type(self): + field = self._make_one('toast', 'INTEGER') + other = object() + self.assertNotEqual(field, other) + self.assertIs(field.__ne__(other), NotImplemented) + + def test___ne___same_value(self): + field1 = self._make_one('test', 'TIMESTAMP', mode='REPEATED') + field2 = self._make_one('test', 'TIMESTAMP', mode='REPEATED') + # unittest ``assertEqual`` uses ``==`` not ``!=``. + comparison_val = (field1 != field2) + self.assertFalse(comparison_val) + + def test___ne___different_values(self): + field1 = self._make_one( + 'test1', 'FLOAT', mode='REPEATED', description='Not same') + field2 = self._make_one( + 'test2', 'FLOAT', mode='NULLABLE', description='Knot saym') + self.assertNotEqual(field1, field2) + def test___hash__set_equality(self): sub1 = self._make_one('sub1', 'STRING') sub2 = self._make_one('sub2', 'STRING')