Skip to content

Add 'Table.row_from_mapping' helper. - #3425

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

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

Conversation

@tseaver

@tseavertseaver commented May 16, 2017

Copy link
Copy Markdown
Contributor

Closes#3396.

@tseavertseaver 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 theacodesMay 16, 2017 19:20
@googlebotgooglebot 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

@dhermesdhermes left a comment

Copy link
Copy Markdown
Contributor

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)

Comment threadbigquery/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

Comment threadbigquery/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

Comment threadbigquery/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

Comment threadbigquery/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

Comment threadbigquery/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

tseaver commented May 17, 2017

Copy link
Copy Markdown
ContributorAuthor

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) formappinginmapping_list])

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

@lukesneeringer

lukesneeringer commented Aug 7, 2017

Copy link
Copy Markdown
Contributor

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

@dhermes

dhermes commented Aug 7, 2017

Copy link
Copy Markdown
Contributor

If LGTY @lukesneeringer then I'm OK with it

@lukesneeringer
lukesneeringer merged commit 1b764b8 into googleapis:masterAug 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
@lukesneeringerlukesneeringer mentioned this pull request Aug 24, 2017
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@theacodestheacodesAwaiting requested review from theacodes
1 more reviewer
@dhermesdhermesdhermes left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: bigqueryIssues related to the BigQuery API.cla: yesThis 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