-
Notifications
You must be signed in to change notification settings - Fork 48
feat: merge
only generates a default index if both inputs already have an index
#733
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.
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
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@@ -61,10 +61,12 @@ def _init_bigquery_thread_local(self): | |||
@property | |||
def bigquery(self) -> bigquery_options.BigQueryOptions: | |||
"""Options to use with the BigQuery engine.""" | |||
if self._local.bigquery_options is not None: | |||
if ( | |||
bigquery_options := getattr(self._local, "bigquery_options", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure how this was failing, but for some reason self._local.bigquery_options
wasn't being initialized when I ran from a notebook in VS Code.
I think it's because the original Options.__init__
from the import ran in a different thread from the one where the actual cell code was running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by #741
@@ -0,0 +1,288 @@ | |||
# Copyright 2024 Google LLC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved from tests/system/small/test_empty_index.py and added test_null_index_merge_*
tests.
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 #<issue_number_goes_here> 🦕