Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

fix: load_table_from_dataframe now assumes there may be local null values - #1735

Merged
gcf-merge-on-green[bot] merged 3 commits into
mainfrom
tswast-patch-2
Nov 22, 2023
Merged

fix: load_table_from_dataframe now assumes there may be local null values#1735
gcf-merge-on-green[bot] merged 3 commits into
mainfrom
tswast-patch-2

Conversation

@tswast

Copy link
Copy Markdown
Contributor

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 and kiraksiNovember 22, 2023 15:33
@product-auto-labelproduct-auto-labelBot 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 kiraksiNovember 22, 2023 15:34

@chalmerlowechalmerlowe left a comment

Copy link
Copy Markdown
Collaborator

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-labelproduct-auto-labelBot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Nov 22, 2023
@tswast

Copy link
Copy Markdown
ContributorAuthor

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
Copy Markdown
ContributorAuthor

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.

@tswasttswast 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-greenBot merged commit f05dc69 into mainNov 22, 2023
@gcf-merge-on-green
gcf-merge-on-greenBot deleted the tswast-patch-2 branch November 22, 2023 16:24
@gcf-merge-on-greengcf-merge-on-greenBot removed the automerge Merge the pull request once unit tests and other checks pass. label Nov 22, 2023
@release-pleaserelease-pleaseBot mentioned this pull request 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)
Fixesgoogleapis#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 freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: bigqueryIssues related to the googleapis/python-bigquery API.size: mPull 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

@tswast@chalmerlowe