Skip to content

Add 'Table.row_from_mapping' helper.#3425

Merged
lukesneeringer merged 2 commits intogoogleapis:masterfrom
tseaver:3396-bigquery-row_from_mapping
Aug 7, 2017
Merged

Add 'Table.row_from_mapping' helper.#3425
lukesneeringer merged 2 commits intogoogleapis:masterfrom
tseaver:3396-bigquery-row_from_mapping

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented May 16, 2017

Closes #3396.

@tseaver tseaver added api: bigquery Issues related to the BigQuery API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels May 16, 2017
@tseaver tseaver requested review from dhermes and theacodes May 16, 2017 19:20
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 16, 2017
dhermes
dhermes reviewed May 16, 2017
View reviewed changes
Copy link
Contributor

@dhermes dhermes left a comment

Choose a reason for hiding this comment

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

I wouldn't say this "closes #3396". That asks us to accept dictionaries in insert_data.

We should either:

  • Explicitly not support it (i.e. either a blacklist or whitelist of types)
  • Accept a polymorphic input (I am not really into that)
  • Offer a sibling insert_data_from_mappings (or a better / shorter name)

bigquery/google/cloud/bigquery/table.py

:rtype: tuple
:returns: Tuple matching the table's schema
:raises: ValueError if table's schema is not set

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

bigquery/google/cloud/bigquery/table.py Outdated
:param mapping: Mapping of row data.

:rtype: tuple
:returns: Tuple matching the table's schema

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

bigquery/google/cloud/bigquery/table.py Outdated
row.append(mapping[field.name])
elif field.mode == 'REPEATED':
row.append(mapping.get(field.name, ()))
else: # NULLABLE

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

bigquery/google/cloud/bigquery/table.py
row.append(mapping.get(field.name, ()))
else: # NULLABLE
row.append(mapping.get(field.name))
return tuple(row)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

bigquery/tests/unit/test_table.py
with self.assertRaises(ValueError) as exc:
table.row_from_mapping(MAPPING)

self.assertEqual(exc.exception.args, (_TABLE_HAS_NO_SCHEMA,))

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver
Copy link
Contributor Author

tseaver commented May 17, 2017

Given a row_from_mapping helper method, the insert_data_from_mappings one is redundant: it is the equivalent of:

table.insert_data([table.row_from_mapping(mapping) for mapping in mapping_list])

I'm proposing closing #3396 with a note to that effect.

@lukesneeringer
Copy link
Contributor

lukesneeringer commented Aug 7, 2017

@dhermes @tseaver This still seems to be in good shape. Can we merge it?

@dhermes
Copy link
Contributor

dhermes commented Aug 7, 2017

If LGTY @lukesneeringer then I'm OK with it

@lukesneeringer lukesneeringer merged commit 1b764b8 into googleapis:master Aug 7, 2017
@tseaver tseaver deleted the 3396-bigquery-row_from_mapping branch August 10, 2017 18:13
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 21, 2017
@tseaver @landrito
Add 'Table.row_from_mapping' helper. (googleapis#3425)
88ccc6f
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
@tseaver @landrito
Add 'Table.row_from_mapping' helper. (googleapis#3425)
0b54db2
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
@tseaver @landrito
Add 'Table.row_from_mapping' helper. (googleapis#3425)
80028a5
@lukesneeringer lukesneeringer mentioned this pull request Aug 24, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@theacodes theacodes Awaiting requested review from theacodes

1 more reviewer

@dhermes dhermes dhermes left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tseaver @lukesneeringer @dhermes @googlebot