Skip to content

Update pyarrow requirement from <8.1.0,>=8.0.0 to >=10.0.1,<10.1.0#1336

Closed
3dbrows wants to merge 15 commits intosnowflakedb:mainfrom
3dbrows:3dbrows/pyarrow-10
Closed

Update pyarrow requirement from <8.1.0,>=8.0.0 to >=10.0.1,<10.1.0#1336
3dbrows wants to merge 15 commits intosnowflakedb:mainfrom
3dbrows:3dbrows/pyarrow-10

Conversation

@3dbrows
Copy link
Copy Markdown
Contributor

@3dbrows 3dbrows commented Nov 17, 2022

This is intended to replace the dependabot PR #1304 by also updating pyproject.toml, and fixing us to one major version.

I am not too sure how to test this against the C++ ArrowIterator. Will the CI test suite be enough?

cc @sfc-gh-sfan .

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 17, 2022

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@3dbrows
Copy link
Copy Markdown
Contributor Author

3dbrows commented Nov 17, 2022

I have read the CLA Document and I hereby sign the CLA

@3dbrows
Copy link
Copy Markdown
Contributor Author

3dbrows commented Nov 21, 2022

Running ci/build_docker.sh produces a lot of C++ build errors so I imagine that does mean that the ArrowIterator would need some refactoring.

@sfc-gh-mkeller sfc-gh-mkeller mentioned this pull request Nov 23, 2022
@noamcohen97
Copy link
Copy Markdown
Contributor

created this branch that builds: #1349

@3dbrows
Copy link
Copy Markdown
Contributor Author

3dbrows commented Nov 25, 2022

Fantastic, thank you @noamcohen97 , I see you upgraded to C++ 17 which is important as Arrow has updated to this recently. I will try including your C++ changes in this branch as well. The maintainers can then choose if they want to close this PR and accept only yours, or accept this one without the Python 3.11 change as well. I am happy either way.

Thank you again.

@noamcohen97
Copy link
Copy Markdown
Contributor

I think if a new version is released, it might as well have new wheels built for 3.11

@noamcohen97
Copy link
Copy Markdown
Contributor

Fantastic, thank you @noamcohen97 , I see you upgraded to C++ 17 which is important as Arrow has updated to this recently. I will try including your C++ changes in this branch as well. The maintainers can then choose if they want to close this PR and accept only yours, or accept this one without the Python 3.11 change as well. I am happy either way.

Thank you again.

would really appreciate a co-authored-by on this :)
I am trying to build up my open source reputation

@3dbrows
Copy link
Copy Markdown
Contributor Author

3dbrows commented Nov 25, 2022

btw @noamcohen97 I found I had to add libarrow_dataset.so.1000 (and equivalents for other platforms) in order for ci/build_docker.sh to pass.

And yep, might as well do 3.11 at the same time.

would really appreciate a co-authored-by on this :)

Yes, absolutely, how do I do this, edit a commit message I guess?

@noamcohen97
Copy link
Copy Markdown
Contributor

noamcohen97 commented Nov 25, 2022

I found I had to add libarrow_dataset.so.1000

Nice!

how do I do this, edit a commit message I guess

Yes, Thank you 😊
You should use the email address

3dbrows and others added 2 commits November 25, 2022 10:33
Co-authored-by: Noam Cohen (noamcohen97) <[email protected]>
Co-authored-by: Noam Cohen (noamcohen97) <[email protected]>
@3dbrows
Copy link
Copy Markdown
Contributor Author

3dbrows commented Nov 25, 2022

@noamcohen97 I think the attribution is fixed now :)

@joshuataylor
Copy link
Copy Markdown

joshuataylor commented Nov 25, 2022

Should it be >= 10.0.1 (for the Python 3.11 support)?

edit: Installing from local whl file on M1 Mac works perfectly :) 🎉

Query ID is 01a88ba8-0402-c4cb-0001-1c831e583396
Hello from python 3.11.0 (main, Oct 25 2022, 13:57:33) [Clang 14.0.0 (clang-1400.0.29.202)]

My poetry file looks like this:

[tool.poetry.dependencies]
python = "^3.11"
snowflake-connector-python = { file = "/Users/josh/dev/snowflake-connector-python/dist/snowflake_connector_python-2.8.3-cp311-cp311-macosx_12_0_arm64.whl" }

@3dbrows
Copy link
Copy Markdown
Contributor Author

3dbrows commented Nov 25, 2022

Very well spotted @joshuataylor , thank you, I've bumped this to 10.0.1.

@3dbrows 3dbrows changed the title Update pyarrow requirement from <8.1.0,>=8.0.0 to >=10.0.0,<10.1.0 Update pyarrow requirement from <8.1.0,>=8.0.0 to >=10.0.1,<10.1.0 Nov 25, 2022
@nattaylor
Copy link
Copy Markdown

Perhaps this is a useless comment, but I checked out 3dbrows/pyarrow-10, built on my M1, and I'm off and running.

@cpcloud
Copy link
Copy Markdown

cpcloud commented Dec 20, 2022

Anything blocking merging this PR?

@3dbrows
Copy link
Copy Markdown
Contributor Author

3dbrows commented Dec 21, 2022

@cpcloud Looks like it's just the description release notes that require updating - should these be updated in this PR, or is that done when the release is cut? If they should be updated now - under what (future) version number?

@sfc-gh-mkeller
Copy link
Copy Markdown
Collaborator

Merged through #1347

@github-actions github-actions bot locked and limited conversation to collaborators Jan 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants