[go: nahoru, domu]

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

chore: consolidate PyarrowVersions helpers #1679

Merged
merged 16 commits into from
Oct 18, 2023
Merged

Conversation

Linchin
Copy link
Contributor
@Linchin Linchin commented Oct 9, 2023

Part of solution for #1430
This PR cleans up the pyarrow version check related code and consolidate them in a single _pyarrow_helpers.py file.

Items:

  • Move PyarrowVersions class to _pyarrow_helpers.py
  • Move pyarrow helper functions into _pyarrow_helpers.py
  • Move pyarrow helper unit tests into test_pyarrow_helpers.py
  • Move bq and arrow type mapping into PyarrowVersions methods
  • Add unit tests for the added methods
  • Delete unit test verifying pyarrow bad version
  • Remove pyarrow bad version check (v2.0.0), because we only allow version 3.0.0 and later
  • Remove installing pyarrow==1.0.0 for python 3.7 in nox unit_noextras session, because the cause (bug: google-cloud-bigquery cannot be imported when pyarrow 2.0 or earlier is installed #933) no longer holds

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)

Will create a separate PR for cleaning up BQStorageVersions.

@Linchin Linchin added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 9, 2023
@Linchin Linchin requested review from a team as code owners October 9, 2023 18:40
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigquery Issues related to the googleapis/python-bigquery API. labels Oct 9, 2023
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Oct 9, 2023
@Linchin Linchin removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 9, 2023
@Linchin Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 11, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 11, 2023
@Linchin Linchin changed the title chore: consolidate pyarrow helpers chore: consolidate PyarrowVersions helpers Oct 11, 2023
@Linchin Linchin requested a review from tswast October 12, 2023 17:56
google/cloud/bigquery/_pyarrow_helpers.py Outdated Show resolved Hide resolved
google/cloud/bigquery/table.py Outdated Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
google/cloud/bigquery/_pyarrow_helpers.py Outdated Show resolved Hide resolved
google/cloud/bigquery/_pyarrow_helpers.py Show resolved Hide resolved
google/cloud/bigquery/client.py Show resolved Hide resolved
tests/unit/test__pyarrow_helpers.py Outdated Show resolved Hide resolved
tests/unit/test_client.py Outdated Show resolved Hide resolved
@Linchin
Copy link
Contributor Author
Linchin commented Oct 13, 2023

After talking to @tswast more about the implementation details, I understood the context and requirements a bit more now:

  • Customers may have installed an earlier version of pyarrow (<v3.0.0). As long as they are not using the part of our code that uses pyarrow (which requires higher version >= v3.0.0), there should not be an error message
  • We raise error messages when the client starts to use pyarrow and finds out that it's version is older than v3.0.0, or pyarrow is not installed at all

@Linchin Linchin requested a review from tswast October 16, 2023 20:27
@Linchin Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 17, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 17, 2023
tests/unit/test__pyarrow_helpers.py Outdated Show resolved Hide resolved
tests/unit/test__versions_helpers.py Outdated Show resolved Hide resolved
google/cloud/bigquery/_versions_helpers.py Outdated Show resolved Hide resolved
google/cloud/bigquery/_versions_helpers.py Outdated Show resolved Hide resolved
google/cloud/bigquery/client.py Outdated Show resolved Hide resolved
Linchin and others added 3 commits October 17, 2023 12:34
Co-authored-by: Tim Swast <swast@google.com>
Co-authored-by: Tim Swast <swast@google.com>
@Linchin Linchin requested review from tswast and removed request for farhan0102 October 17, 2023 23:53
@tswast tswast merged commit f637e5e into googleapis:main Oct 18, 2023
17 checks passed
@Linchin Linchin deleted the pyarrow branch October 30, 2023 23:37
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: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants