feat: allow setting table labels in to_gbq
#941
Conversation
- add myself to OWNERS - done to test if I set up
This reverts commit 6257ad7.
to_gbqto_gbq
bigframes/dataframe.py
Outdated
| @@ -3106,6 +3107,12 @@ def to_gbq( | ||
| + constants.DEFAULT_EXPIRATION, | ||
| ) | ||
|
|
||
| if len(labels) != 0: | ||
| client = self._session.bqclient |
There was a problem hiding this comment.
Setting up labels currently requires additional client calls, which is acceptable but not ideal for performance. Could you try setting the label directly at the bigquery.table.TableReference destination? While TableReference doesn't have a public API for this, you can experiment with the internal call similar to Table.label. If this works, we can explore adding a new public API to TableReference. +@tswast has more experience with the Python client API and could provide additional insights.
There was a problem hiding this comment.
I removed one client call by using the TableReference returned by the Query job, but as far as I can tell we might still have to make another API call for update_table. Looking at seeing if I can change table labels upon creation but I haven't found where that's being done yet.
There was a problem hiding this comment.
Thanks for exploring it.
| @@ -4657,6 +4657,17 @@ def test_to_gbq_and_create_dataset(session, scalars_df_index, dataset_id_not_cre | ||
| assert not loaded_scalars_df_index.empty | ||
|
|
||
|
|
||
| def test_to_gbq_table_labels(scalars_df_index): | ||
| destination_table = "bigframes-dev.bigframes_tests_sys.table_labels" |
There was a problem hiding this comment.
Ideally the test should not be hard coupled with a project/dataset. Do we have to use a fixed table name? I think the test would still serve its purpose if we did to_gbq without an explicit table arg and asserted labels on the returned anonymous table.
| @@ -467,6 +468,9 @@ def to_gbq( | ||
| clustering order within the Index/DataFrame columns follows the order | ||
| specified in `clustering_columns`. | ||
|
|
||
| labels (dict[str, str], default None): | ||
| Specifies table labels within BigQuery |
There was a problem hiding this comment.
nit, (but since this goes in the public doc) should end with the period .
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes b/312727426 🦕