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

deps: use pandas-gbq to determine schema in load_table_from_dataframe #2095

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

tswast
Copy link
Contributor

@tswast tswast commented Dec 26, 2024

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)

Towards go/pandas-gbq-and-bigframes-redundancy
🦕

@tswast tswast requested review from a team as code owners December 26, 2024 22:28
@tswast tswast requested a review from chalmerlowe December 26, 2024 22:28
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Dec 26, 2024
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Dec 26, 2024
@tswast
Copy link
Contributor Author

tswast commented Dec 27, 2024

tests/unit/test_client.py::TestClientUpload::test_load_table_from_dataframe_w_higher_scale_decimal128_datatype looks like a real failure and might need a change in pandas-gbq.

@tswast
Copy link
Contributor Author

tswast commented Jan 3, 2025

I've tested locally with googleapis/python-bigquery-pandas#844 and that PR should fix the remaining system test.

(venv) ➜  python-bigquery git:(b323176126-pandas-gbq) pytest tests/unit/test_client.py::TestClientUpload::test_load_table_from_dataframe_w_higher_scale_decimal128_datatype
==================================================== test session starts ====================================================
platform linux -- Python 3.12.6, pytest-8.3.4, pluggy-1.5.0
rootdir: /home/swast/src/github.com/googleapis/python-bigquery
configfile: pyproject.toml
collected 1 item                                                                                                            

tests/unit/test_client.py .                                                                                           [100%]

===================================================== 1 passed in 0.08s =====================================================

pyproject.toml Outdated Show resolved Hide resolved
@tswast tswast changed the title feat: use pandas-gbq to determine schema in load_table_from_dataframe deps: use pandas-gbq to determine schema in load_table_from_dataframe Jan 7, 2025
@tswast tswast requested a review from Linchin January 7, 2025 15:58
@tswast
Copy link
Contributor Author

tswast commented Jan 22, 2025

test_load_table_from_dataframe_w_partial_schema_extra_types

Looks like a real failure (the warning message changed slightly in pandas-gbq). I'll try to update the test to be a bit more robust.

@tswast tswast added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 23, 2025
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 23, 2025
@tswast
Copy link
Contributor Author

tswast commented Jan 23, 2025

@Linchin @chalmerlowe I believe this is finally ready for review. The failing checks are for infrastructure / Kokoro problems for a new setup, possibly?

@@ -1259,7 +1259,7 @@ def test_upload_time_and_datetime_56(bigquery_client, dataset_id):
df = pandas.DataFrame(
dict(
dt=[
datetime.datetime(2020, 1, 8, 8, 0, 0),
datetime.datetime(2020, 1, 8, 8, 0, 0, tzinfo=datetime.timezone.utc),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure is we should have defined what happens when you mix and match naive datetime (DATETIME in BQ) with datetime + timezone (TIMESTAMP in BQ). Since the test expects TIMESTAMP, I update this to be a datetime + timezone.

We still mix/match timezones, so I think that part is important to make sure we end up returning UTC values.

Copy link
Collaborator

@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.

By and large, this looks good.
I have a question that might be a potential blocker.

google/cloud/bigquery/_pandas_helpers.py Outdated Show resolved Hide resolved
@@ -74,6 +74,9 @@ bqstorage = [
]
pandas = [
"pandas >= 1.1.0",
"pandas-gbq >= 0.26.1; python_version >= '3.8'",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this means we are testing without pandas-gbq on Python 3.7.

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. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants