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.

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

to_dataframe always shows warning if max_results is set #806

Closed
tswast opened this issue Jul 23, 2021 · 2 comments · Fixed by #815
Closed

to_dataframe always shows warning if max_results is set #806

tswast opened this issue Jul 23, 2021 · 2 comments · Fixed by #815
Assignees
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@tswast
Copy link
Contributor

tswast commented Jul 23, 2021

to_dataframe always shows a warning that bqstorage_client is set if max_results is set. This warning should not be shown if bqstorage_client is not explicitly set.

Code example

df = bigquery_client.list_rows(
        scalars_table,
        max_results=10,
)

Stack trace

$ pytest tests/system/test_pandas.py::test_list_rows_nullable_scalars_dtypes
========================================= test session starts =========================================
platform darwin -- Python 3.9.5, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /Users/swast/src/python-bigquery
plugins: cov-2.12.1, asyncio-0.15.1
collected 2 items                                                                                     

tests/system/test_pandas.py ..                                                                  [100%]

========================================== warnings summary ===========================================
tests/system/test_pandas.py::test_list_rows_nullable_scalars_dtypes[10]
  /Users/swast/src/python-bigquery/google/cloud/bigquery/table.py:1899: UserWarning: Cannot use bqstorage_client if max_results is set, reverting to fetching data with the REST endpoint.
    if not self._validate_bqstorage(bqstorage_client, create_bqstorage_client):

-- Docs: https://docs.pytest.org/en/stable/warnings.html
==================================== 2 passed, 1 warning in 11.95s ====================================
@tswast tswast added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jul 23, 2021
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Jul 23, 2021
@plamut plamut self-assigned this Jul 26, 2021
@plamut
Copy link
Contributor

plamut commented Jul 26, 2021

Simply patching _validate_bqstorage() is troublesome, because there are many paths that directly or indirectly access this helper.

We'll have to move the max_results + BQ Storage client check out of that method and and perform it at all places where user code could call a public method and explicitly pass in a BQ Storage client instance. Everything else seems to result in duplicate warnings/false warnings/no warnings at all.

@tswast
Copy link
Contributor Author

tswast commented Jul 26, 2021

Yeah, warning just in the public methods probably makes the most sense.

On the plus side, that'd allow the stacklevel argument to warn to work correctly, showing the actually line of the user's code that caused the warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants