Skip to content

fix: exclude DataFrame and Series __call__ from unimplemented API metrics #1351

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

Merged
merged 2 commits into from
Feb 3, 2025

Conversation

tswast
Copy link
Collaborator

@tswast tswast commented Feb 3, 2025

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:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes internal issue 393769125 🦕

@tswast tswast requested review from a team as code owners February 3, 2025 14:53
@tswast tswast requested a review from chelsea-lin February 3, 2025 14:53
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. labels Feb 3, 2025
@tswast
Copy link
Collaborator Author

tswast commented Feb 3, 2025

Doctest looks like a real failure:

_______________________ [doctest] frame.DataFrame.update _______________________
[gw3] linux -- Python 3.12.7 /tmpfs/src/github/python-bigquery-dataframes/.nox/doctest/bin/python
4198         **Examples:**
4199 
4200             >>> import bigframes.pandas as bpd
4201             >>> bpd.options.display.progress_bar = None
4202 
4203             >>> df = bpd.DataFrame({'A': [1, 2, 3],
4204             ...                    'B': [400, 500, 600]})
4205             >>> new_df = bpd.DataFrame({'B': [4, 5, 6],
4206             ...                        'C': [7, 8, 9]})
4207             >>> df.update(new_df)
UNEXPECTED EXCEPTION: TypeError('Series is not callable. Did you mean to use square brackets (e.g. series[row_label]), instead? Share your usecase with the BigQuery DataFrames team at the [https://bit.ly/bigframes-feedback](https://www.google.com/url?q=https://bit.ly/bigframes-feedback&sa=D) survey.You are currently running BigFrames version 1.34.0')
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/doctest.py", line 1368, in __run
    exec(compile(example.source, filename, "single",
  File "<doctest frame.DataFrame.update[4]>", line 1, in <module>
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 161, in wrapper
    raise e
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 145, in wrapper
    return method(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/dataframe.py", line 1217, in update
    result = self.combine(other, update_func, how=join)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 161, in wrapper
    raise e
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 145, in wrapper
    return method(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/dataframe.py", line 1248, in combine
    result = func(lseries, rseries)
             ^^^^^^^^^^^^^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/dataframe.py", line 1208, in update_func
    return left.mask(right.notna(), right)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 161, in wrapper
    raise e
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 145, in wrapper
    return method(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/series.py", line 1729, in mask
    cond = self.apply(cond, by_row=False)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 161, in wrapper
    raise e
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 145, in wrapper
    return method(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/series.py", line 1536, in apply
    return func(self)
           ^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 161, in wrapper
    raise e
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/core/log_adapter.py", line 145, in wrapper
    return method(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmpfs/src/github/python-bigquery-dataframes/bigframes/series.py", line 229, in __call__
    raise TypeError(
TypeError: Series is not callable. Did you mean to use square brackets (e.g. series[row_label]), instead? Share your usecase with the BigQuery DataFrames team at the [https://bit.ly/bigframes-feedback](https://www.google.com/url?q=https://bit.ly/bigframes-feedback&sa=D) survey.You are currently running BigFrames version 1.34.0
/[tmpfs/src/github/python-bigquery-dataframes/third_party/bigframes_vendored/pandas/core/frame.py:4207]
@tswast
Copy link
Collaborator Author

tswast commented Feb 3, 2025

Confirmed that pytest tests/system/small/test_dataframe.py::test_df_update fails, too. I'll investigate further.

@tswast
Copy link
Collaborator Author

tswast commented Feb 3, 2025

Unfortunately, by implementing __call__, we make callable return true: https://docs.python.org/3/library/functions.html#callable

I think we just need to remove this from our tracking.

@tswast tswast changed the title fix: add feedback link to DataFrame and Series __call__ error Feb 3, 2025
@tswast
Copy link
Collaborator Author

tswast commented Feb 3, 2025

Updated PR to remove __call__ from tracking instead of implementing it.

@tswast
Copy link
Collaborator Author

tswast commented Feb 3, 2025

Since tests pass on other Python versions, the notebook failures seem unrelated:

Failed: remote_functions/remote_function_vertex_claude_model.ipynb
nox > Command python scripts/run_and_publish_benchmark.py --notebook --publish-benchmarks=notebooks/ failed with exit code 1
nox > Session notebook-3.12 failed.
nox > Ran multiple sessions:
nox > * notebook-3.9: success
nox > * notebook-3.10: success
nox > * notebook-3.12: failed

Edit: mailed #1353 to reduce notebook tests to only the most important Python versions to help avoid some of these quota issues.

@tswast tswast merged commit f2d5264 into main Feb 3, 2025
22 of 23 checks passed
@tswast tswast deleted the b393769125-__call__ branch February 3, 2025 18:13
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-dataframes API. size: s Pull request size is small.
3 participants