Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: reconfigure tqdm progress bar in %%bigquery magic #1355

Merged
merged 51 commits into from
Oct 11, 2022
Merged

feat: reconfigure tqdm progress bar in %%bigquery magic #1355

merged 51 commits into from
Oct 11, 2022

Conversation

aribray
Copy link
Contributor

@aribray aribray commented Sep 15, 2022 •
edited
Loading

Change-Id: I2add62e3cdd5f25f88ace2d08f212796918158b6

Adds the BigQuery job ID to the tqdm progress bar description for a more helpful user message. Adds the correct tqdm.notebook import. Changes the default progress bar to tqdm_notebook

fixes: #1378
See internal 242869795

@aribray
feat: add bigquery job id to tqdm progress bar description
ac10e76 
Change-Id: I2add62e3cdd5f25f88ace2d08f212796918158b6
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: bigquery Issues related to the googleapis/python-bigquery API. labels Sep 15, 2022
@aribray aribray marked this pull request as ready for review September 15, 2022 18:07
@aribray aribray requested a review from a team September 15, 2022 18:07
@aribray aribray requested a review from a team as a code owner September 15, 2022 18:07
@aribray aribray self-assigned this Sep 15, 2022
@aribray
write to sys.stdout instead of sys.stderr
9db15fc 
Change-Id: I6c4001608af1bd8c305c53c6089d64f99605bd8c
@aribray aribray changed the title feat: add bigquery job id to tqdm progress bar description feat: reconfigure tqdm progress bar in %%bigquery magic Sep 23, 2022
@aribray
Merge remote-tracking branch 'upstream/main' into aribray--tqdm
6ed2f03 
Change-Id: I2627d3ee386b59abd427d2837c83e9f92ec1f35b
@aribray
configure progress bar
c3ec846 
Change-Id: I5788448d580b53898e75fba68ff5d5a9d12e33d6
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Sep 23, 2022
@aribray
tqdm.notebook
f50bf8b 
Change-Id: I87e45085b7535083327a5fe2e51dba4b6411db00
@aribray aribray added the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 23, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 23, 2022
gcf-owl-bot bot and others added 5 commits September 23, 2022 17:03
@gcf-owl-bot
🦉 Updates from OwlBot post-processor
0c41c9e 
See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md
@aribray
reinclude ipywidgets
d679134 
Change-Id: Ibe0fc01db05fcfaacdbe0c074b841ead3a39afc9
@aribray
Merge branch 'aribray--tqdm' of github.com:aribray/python-bigquery in…
87212a5 
…to aribray--tqdm

Change-Id: I6448d5fb1d6a4e91a405b93b099ac00d748087d8
@aribray
reinclude ipywidgets
be991b7 
Change-Id: I56f8f98853e83ead0e0ca743c03407a521370233
@aribray
change test assertions to tqdm_notebook
c6317a4 
Change-Id: I2d55e529142ad0024ef4a98c2f15d10a73535380
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Sep 23, 2022
@aribray
change test assertions in test_magics
13331b9 
Change-Id: I7961ff1c5e9c54930d077e67ef9e01d79e351c5f
@aribray
remove ipywidgets
d86ae58 
Change-Id: I183e277fc7be8797c85d6802f4f8c3947871d4cc
@aribray
update assertions in test
a6c3e88 
Change-Id: I3b4a1b9460227ca49bf344362efbcc2c895d804d
@aribray aribray added the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 23, 2022
@aribray aribray added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 10, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 10, 2022
shollyman
shollyman approved these changes Oct 11, 2022
View reviewed changes
Copy link
Contributor

@shollyman shollyman left a comment

Choose a reason for hiding this comment

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

One minor question about the helpers, otherwise LGTM.

google/cloud/bigquery/_tqdm_helpers.py Show resolved Hide resolved
@aribray aribray merged commit 506f781 into googleapis:main Oct 11, 2022
@release-please release-please bot mentioned this pull request Oct 11, 2022
Merged
tswast
tswast reviewed Oct 12, 2022
View reviewed changes
Copy link
Contributor

@tswast tswast left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks so much for working through those testing issues. Just one nit, that might be worth a little follow up

google/cloud/bigquery/_tqdm_helpers.py
)
try:
query_result = query_job.result(
timeout=_PROGRESS_BAR_UPDATE_INTERVAL, max_results=max_results
)
progress_bar.update(default_total)
progress_bar.set_description(
"Query complete after {:0.2f}s".format(time.time() - start_time),
f"Job ID {query_job.job_id} successfully executed",
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to include the location in the output too, formatted like the bq cli as location:I'd https://cloud.google.com/python/docs/reference/bigquery/latest/google.cloud.bigquery.job.QueryJob#google_cloud_bigquery_job_QueryJob_location

I believe the location is required to fetch the job metadata for the non-default location.

chalmerlowe
chalmerlowe reviewed Oct 12, 2022
View reviewed changes
Copy link
Contributor

@chalmerlowe chalmerlowe left a comment

Choose a reason for hiding this comment

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

A couple of minor tweaks and a question.
Other than that, looks good to me.

google/cloud/bigquery/magics/magics.py
@@ -286,7 +286,7 @@ def _handle_error(error, destination_var=None):

Args:
error (Exception):
An exception that ocurred during the query exectution.
An exception that ocurred during the query execution.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change the word ocurred to the correct spelling of occurred

An exception that occurred during the query execution.

tests/system/test_magics.py
@@ -71,8 +71,7 @@ def test_bigquery_magic(ipython_interactive):
# Removes blanks & terminal code (result of display clearing)
updates = list(filter(lambda x: bool(x) and x != "\x1b[2K", lines))
assert re.match("Executing query with job ID: .*", updates[0])
assert all(re.match("Query executing: .*s", line) for line in updates[1:-1])
assert re.match("Query complete after .*s", updates[-1])
assert (re.match("Query executing: .*s", line) for line in updates[1:-1])
Copy link
Contributor

Choose a reason for hiding this comment

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

Using assert with a tuple will always return True, no matter what is in the tuple unless the tuple is empty.
Are we trying to detect whether any matches are True OR are we trying to detect whether ALL the responses are True (...as implied by the deleted code above)?

In [9]: assert (True, True, True)
<>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
<ipython-input-9-0a5188c9e554>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
  assert (True, True, True)

In [10]: assert (False, False, False)
<>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
<ipython-input-10-f31b4c57c272>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
  assert (False, False, False)

In [11]: assert (False, False, True)
<>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
<ipython-input-11-59e8f7bc6fa4>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
  assert (False, False, True)

In [14]: assert (True,)
<>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
<ipython-input-14-7b46914816ee>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
  assert (True,)

In [15]: assert (False,)
<>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
<ipython-input-15-a62a5cb79bea>:1: SyntaxWarning: assertion is always true, perhaps remove parentheses?
  assert (False,)
  
  In [16]: assert ()
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
Input In [16], in <cell line: 1>()
----> 1 assert ()

This was referenced Nov 8, 2022
Closed
Closed
Closed
abdelmegahedgoogle pushed a commit to abdelmegahedgoogle/python-bigquery that referenced this pull request Apr 17, 2023
@aribray @gcf-owl-bot @abdelmegahedgoogle
feat: reconfigure tqdm progress bar in %%bigquery magic (googleapis#1355
8405b1c 
)

* feat: add bigquery job id to tqdm progress bar description

Change-Id: I2add62e3cdd5f25f88ace2d08f212796918158b6

* write to sys.stdout instead of sys.stderr

Change-Id: I6c4001608af1bd8c305c53c6089d64f99605bd8c

* configure progress bar

Change-Id: I5788448d580b53898e75fba68ff5d5a9d12e33d6

* tqdm.notebook

Change-Id: I87e45085b7535083327a5fe2e51dba4b6411db00

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* reinclude ipywidgets

Change-Id: Ibe0fc01db05fcfaacdbe0c074b841ead3a39afc9

* reinclude ipywidgets

Change-Id: I56f8f98853e83ead0e0ca743c03407a521370233

* change test assertions to tqdm_notebook

Change-Id: I2d55e529142ad0024ef4a98c2f15d10a73535380

* change test assertions in test_magics

Change-Id: I7961ff1c5e9c54930d077e67ef9e01d79e351c5f

* remove ipywidgets

Change-Id: I183e277fc7be8797c85d6802f4f8c3947871d4cc

* update assertions in test

Change-Id: I3b4a1b9460227ca49bf344362efbcc2c895d804d

* update method args in query.py and table.py

Change-Id: I9a2bf2b54579668ff36ed992e599f4c7fabe918c

* string formatting

* fix typo

* fix incorrect import structure for tqdm notebook

* change default decorator back to tqdm

* modify system test

* add ipywidgets package for tqdm.notebook feature, set tqdm.notebook as default decorator for bq magic

* change test assertion in test_query_pandas

* revert test changes

* reformat import statement

* reformat import statement

* remove timeouterror side effect

* add tqdm mock patch

* Revert "reformat import statement"

This reverts commit 4114221.

* Revert "add tqdm mock patch"

This reverts commit ef809a0.

* add timeout side effect

* fix assertion

* fix import

* change mock patch to tqdm

* move assertion

* move assertions

* add timeout side effect

* adjust import statement, mock.patch tqdm

* create fixture

* revert import change

* add import from helper

* fix linting

* remove unused imort

* set ipywidgets version to 7.7.1

* set ipywidgets version to 7.7.1

* set ipywidgets version to 7.7.1

* bump sphinx version

* bump sphinx version

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalmerlowe chalmerlowe chalmerlowe left review comments

@tswast tswast tswast left review comments

@shollyman shollyman shollyman approved these changes

@alvarowolfx alvarowolfx Awaiting requested review from alvarowolfx

@loferris loferris Awaiting requested review from loferris loferris is a code owner automatically assigned from googleapis/python-samples-owners

@leahecole leahecole Awaiting requested review from leahecole

Assignees

@aribray aribray

Labels
api: bigquery Issues related to the googleapis/python-bigquery API. size: m Pull request size is medium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

module 'tqdm' has no attribute 'notebook' error
6 participants
@aribray @tswast @shollyman @leahecole @chalmerlowe @yoshi-kokoro