8000 fix: `load_table_from_dataframe` now assumes there may be local null values by tswast · Pull Request #1735 · googleapis/python-bigquery · GitHub
[go: up one dir, main page]

Skip to content

Conversation

tswast
Copy link
Contributor
@tswast tswast commented Nov 22, 2023

Even if the remote schema is REQUIRED

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 #1692 🦕

…values

Even if the remote schema is REQUIRED
@tswast tswast requested review from a team as code owners November 22, 2023 15:33
@tswast tswast requested a review from kiraksi November 22, 2023 15:33
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/python-bigquery API. labels Nov 22, 2023
@tswast tswast requested review from chalmerlowe and removed request for kiraksi November 22, 2023 15:34
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.

LGTM
We likely need a unit test or modification to a unit test to cover the change to the return value.

@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 Nov 22, 2023
@tswast
Copy link
Contributor Author
tswast commented Nov 22, 2023

We likely need a unit test or modification to a unit test to cover the change to the return value

@chalmerlowe Agreed! I've updated the unit tests as well as added a system test that demonstrated the issue to be used as a regression test.

assert field.mode == "REQUIRED"


def test_load_table_from_dataframe_w_required_but_local_nulls_fails(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've verified on origin/main that this test currently fails with FAILED tests/system/test_pandas.py::test_load_table_from_dataframe_w_required_but_local_nulls_fails - Failed: DID NOT RAISE <class 'google.api_core.exceptions.BadRequest'> but passes after this fix.

@tswast tswast added the automerge Merge the pull request once unit tests and other checks pass. label Nov 22, 2023
@gcf-merge-on-green gcf-merge-on-green bot merged commit f05dc69 into main Nov 22, 2023
@gcf-merge-on-green gcf-merge-on-green bot deleted the tswast-patch-2 branch November 22, 2023 16:24
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Nov 22, 2023
kiraksi pushed a commit to kiraksi/python-bigquery that referenced this pull request Nov 27, 2023
…values (googleapis#1735)

Even if the remote schema is REQUIRED

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](https://togithub.com/googleapis/python-bigquery/issues/new/choose) 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 googleapis#1692 🦕
kiraksi added a commit that referenced this pull request Nov 28, 2023
…nto pandas extra (#1726)

* feat: Introduce compatibility with native namespace packages

* Update copyright year

* removed pkg_resources from all test files and moved importlib into pandas extra

* feat: removed pkg_resources from all test files and moved importlib into pandas extra

* Adding no cover tag to test code

* reformatted with black

* undo revert

* perf: use the first page a results when `query(api_method="QUERY")` (#1723)

* perf: use the first page a results when `query(api_method="QUERY")`

* add tests

* respect max_results with cached page

* respect page_size, also avoid bqstorage if almost fully downloaded

* skip true test if bqstorage not installed

* coverage

* fix: ensure query job retry has longer deadline than API request deadline (#1734)

In cases where we can't disambiguate API failure from job failure,
this ensures we can still retry the job at least once.

* fix: `load_table_from_dataframe` now assumes there may be local null values (#1735)

Even if the remote schema is REQUIRED

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](https://togithub.com/googleapis/python-bigquery/issues/new/choose) 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 #1692 🦕

* chore: standardize samples directory - delete unneeded dependencies (#1732)

* chore: standardize samples directory = delete unneeded dependencies

* Removed unused import for linter

* fix: move grpc, proto-plus and protobuf packages to extras (#1721)

* chore: move grpc, proto-plus and protobuff packages to extras

* formatted with black

* feat: add `job_timeout_ms` to job configuration classes (#1675)

* fix: adds new property and tests

* 🦉 Updates from OwlBot post-processor

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

* updates docs to correct a sphinx failure

* Updates formatting

* Update tests/system/test_query.py

* 🦉 Updates from OwlBot post-processor

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

* Update google/cloud/bigquery/job/base.py

* updates one test and uses int_or_none

* Update tests/system/test_query.py

testing something.

* Update tests/system/test_query.py

* testing coverage feature

* 🦉 Updates from OwlBot post-processor

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

* minor edits

* tweaks to noxfile for testing purposes

* add new test to base as experiment

* adds a test, updates import statements

* add another test

* edit to tests

* formatting fixes

* update noxfile to correct debug code

* removes unneeded comments.

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

---------

Co-authored-by: Chalmer Lowe <chalmerlowe@google.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Tim Swast <swast@google.com>

* remove unnecessary version checks

* undo bad commit, remove unneeded version checks

* Revert "undo bad commit, remove unneeded version checks"

This reverts commit 5c82dcf.

* Revert "remove unnecessary version checks"

This reverts commit 9331a7e.

* revert bad changes, remove pkg_resources from file

* after clarification, reimplement changes and ignore 3.12 tests

* reformatted with black

* removed minimum check

* updated pandas installed version check

---------

Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Tim Swast <swast@google.com>
Co-authored-by: Chalmer Lowe <chalmerlowe@google.com>
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

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.

load_table_from_dataframe does not error out when nan in a required column - Million dollar bug

2 participants

0