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.

Sign up for GitHub

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

Jump to bottom

Create subsets of pyarrow package with pyarrow-core < pyarrow < pyarrow-all and update to Arrow v16.0.0 #1376

Merged
merged 14 commits into from
May 7, 2024
Merged

Create subsets of pyarrow package with pyarrow-core < pyarrow < pyarrow-all and update to Arrow v16.0.0 #1376

merged 14 commits into from
May 7, 2024

Conversation

raulcd
Copy link
Member

@raulcd raulcd commented Apr 29, 2024 •
edited by h-vetinari
Loading

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Continuation of #1255

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@raulcd raulcd mentioned this pull request Apr 29, 2024
Closed
5 tasks
raulcd
raulcd commented Apr 29, 2024
View reviewed changes
recipe/meta.yaml Outdated Show resolved Hide resolved
@raulcd raulcd force-pushed the add-pyarrow-base branch from b8e17a8 to b39a6f3 Compare April 30, 2024 06:57
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@raulcd raulcd force-pushed the add-pyarrow-base branch from b39a6f3 to 1443361 Compare April 30, 2024 06:59
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@raulcd raulcd force-pushed the add-pyarrow-base branch from 1443361 to a632f02 Compare April 30, 2024 07:31
@amoeba amoeba mentioned this pull request May 1, 2024
Merged
h-vetinari
h-vetinari reviewed May 2, 2024
View reviewed changes
Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

This is getting close, thanks a lot! Just some minor fixes left.

We should also revert efd6784 & 985aeec (already on 15.x, but I can cherry-pick those reverts if we end up merging this PR first) and remove the comments referring to the caps.

recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
@raulcd raulcd force-pushed the add-pyarrow-base branch from da9c6e4 to 54db236 Compare May 2, 2024 09:54
@raulcd
Copy link
Member Author

raulcd commented May 2, 2024

We should also revert efd6784 & 985aeec (already on 15.x, but I can cherry-pick those reverts if we end up merging this PR first) and remove the comments referring to the caps.

I can revert those but I am not sure what you mean by already on 15.x. I can't see the reverts on main, so I'll let you do the cherry-pick here :)

@h-vetinari h-vetinari mentioned this pull request May 3, 2024
Closed
@h-vetinari
Copy link
Member

This PR is now ready from my side. However, unless there's very concrete urgency, I'd like to wait a couple of days until people have had a chance to respond to #1381, which would simply entail moving all the python-* outputs here to https://github.com/conda-forge/pyarrow-feedstock. That's not complicated per se, but it would make the transition much easier IMO if this happened at a version boundary.

@h-vetinari
Copy link
Member

Ah actually, something is still amiss. We're not creating different hashes between the CUDA & non-CUDA python wrappers. That wouldn't be an issue per se (especially if we do #1381), but it is an issue since we're currently doing pin_subpackage(..., exact=True), which means it will be a random race which of the two python wrappers gets uploaded first (per python version & platform):

linux-64 non-CUDA linux-64 CUDA
pyarrow-core-16.0.0-py38hc396e17_0_cpu.conda pyarrow-core-16.0.0-py38h5350213_0_cuda.conda distinct ✅
pyarrow-core-16.0.0-py39h38d04b8_0_cpu.conda pyarrow-core-16.0.0-py39hb79a567_0_cuda.conda distinct ✅
pyarrow-core-16.0.0-py310hd207890_0_cpu.conda pyarrow-core-16.0.0-py310h40d6bed_0_cuda.conda distinct ✅
pyarrow-core-16.0.0-py311hd5e4297_0_cpu.conda pyarrow-core-16.0.0-py311h373265a_0_cuda.conda distinct ✅
pyarrow-core-16.0.0-py312h3f82784_0_cpu.conda pyarrow-core-16.0.0-py312h0354947_0_cuda.conda distinct ✅
pyarrow-16.0.0-py38hb563948_0.conda pyarrow-16.0.0-py38hb563948_0.conda matches ❌
pyarrow-16.0.0-py39h8003fee_0.conda pyarrow-16.0.0-py39h8003fee_0.conda matches ❌
pyarrow-16.0.0-py310h17c5347_0.conda pyarrow-16.0.0-py310h17c5347_0.conda matches ❌
pyarrow-16.0.0-py311h781c19f_0.conda pyarrow-16.0.0-py311h781c19f_0.conda matches ❌
pyarrow-16.0.0-py312h8da182e_0.conda pyarrow-16.0.0-py312h8da182e_0.conda matches ❌
pyarrow-all-16.0.0-py38hb563948_0.conda pyarrow-all-16.0.0-py38hb563948_0.conda matches ❌
pyarrow-all-16.0.0-py39h8003fee_0.conda pyarrow-all-16.0.0-py39h8003fee_0.conda matches ❌
pyarrow-all-16.0.0-py310h17c5347_0.conda pyarrow-all-16.0.0-py310h17c5347_0.conda matches ❌
pyarrow-all-16.0.0-py311h781c19f_0.conda pyarrow-all-16.0.0-py311h781c19f_0.conda matches ❌
pyarrow-all-16.0.0-py312h8da182e_0.conda pyarrow-all-16.0.0-py312h8da182e_0.conda matches ❌
pyarrow-tests-16.0.0-py38hc396e17_0_cpu.conda pyarrow-tests-16.0.0-py38h5350213_0_cuda.conda distinct ✅
pyarrow-tests-16.0.0-py39h38d04b8_0_cpu.conda pyarrow-tests-16.0.0-py39hb79a567_0_cuda.conda distinct ✅
pyarrow-tests-16.0.0-py310hd207890_0_cpu.conda pyarrow-tests-16.0.0-py310h40d6bed_0_cuda.conda distinct ✅
pyarrow-tests-16.0.0-py311hd5e4297_0_cpu.conda pyarrow-tests-16.0.0-py311h373265a_0_cuda.conda distinct ✅
pyarrow-tests-16.0.0-py312h3f82784_0_cpu.conda pyarrow-tests-16.0.0-py312h0354947_0_cuda.conda distinct ✅

@h-vetinari
Copy link
Member

Ugh, the move from CUDA 11.2 -> 11.8 seems to have caused the aarch builds to hang (CC @jakirkham), and the non-CUDA build on ppc is now crashing in the test suite. No idea if this could be related to pytest 8 somehow, the stacktrace is useless.

@raulcd
Copy link
Member Author

raulcd commented May 3, 2024

No idea if this could be related to pytest 8 somehow, the stacktrace is useless.

I don't think it is as in my last commit (removing the pytest pin) ppc was successful, there was only a flaky test failure on aarch64

@h-vetinari
Copy link
Member

I don't think it is as in my last commit (removing the pytest pin) ppc was successful,

Are you sure? Because 99dbba1 came after your last commit.

@raulcd
Copy link
Member Author

raulcd commented May 3, 2024

Are you sure? Because 99dbba1 came after your last commit.

True, I forgot that I didn't push the revert even though I did it locally. I've done so many tests and changes on this PR that I should give it some time :)

@timkpaine timkpaine mentioned this pull request May 3, 2024
Merged
@h-vetinari
Copy link
Member

Turns out it's related to the compiler version. Both GCC 11 (newly pulled in because CUDA 11.8 allows it) breaks on linux-aarch64 and GCC 12 leads to problems on ppc (like it did on aarch).

h-vetinari
h-vetinari approved these changes May 4, 2024
View reviewed changes
Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

OK, this is ready for merging now. I'd still like to hear some feedback on #1381, but if we cannot come to a decision there quickly, I'm OK to merge this as-is and deal with the split later.

@h-vetinari h-vetinari mentioned this pull request May 7, 2024
Merged
5 tasks
@h-vetinari h-vetinari mentioned this pull request May 7, 2024
Merged
@h-vetinari h-vetinari merged commit 4644239 into conda-forge:main May 7, 2024
12 checks passed
amoeba added a commit to apache/arrow that referenced this pull request May 16, 2024
@amoeba @jorisvandenbossche
GH-41105: [Python][Docs] Update PyArrow installation docs for conda p…
f5ac05c 
…ackage split (#41135)

Do not merge until some discussion is had about how to time this relative to conda-forge/arrow-cpp-feedstock#1376. Additionally, consider hot-patching this into arrow-site if appropriate.

### What changes are included in this PR?

Updates to the [Python installation docs](https://arrow.apache.org/docs/python/install.html) to reflect the in-progress change splitting PyArrow on conda-forge into three separate packages. Specifically:

1. Add a note in the conda section highlighting that there are three packages and linking to a new section (2) in order to provide more information
2. Add a new section, linked from (1), providing a comparison of each package as a table

### Are these changes tested?

These are just docs changes. I have built them locally and they look fine.

### Are there any user-facing changes?

Just docs.
* GitHub Issue: #41105

Lead-authored-by: Bryce Mecum <[email protected]>
Co-authored-by: Joris Van den Bossche <[email protected]>
Signed-off-by: Bryce Mecum <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this pull request May 25, 2024
@amoeba @jorisvandenbossche @vibhatha
apacheGH-41105: [Python][Docs] Update PyArrow installation docs for c…
e348997 
…onda package split (apache#41135)

Do not merge until some discussion is had about how to time this relative to conda-forge/arrow-cpp-feedstock#1376. Additionally, consider hot-patching this into arrow-site if appropriate.

### What changes are included in this PR?

Updates to the [Python installation docs](https://arrow.apache.org/docs/python/install.html) to reflect the in-progress change splitting PyArrow on conda-forge into three separate packages. Specifically:

1. Add a note in the conda section highlighting that there are three packages and linking to a new section (2) in order to provide more information
2. Add a new section, linked from (1), providing a comparison of each package as a table

### Are these changes tested?

These are just docs changes. I have built them locally and they look fine.

### Are there any user-facing changes?

Just docs.
* GitHub Issue: apache#41105

Lead-authored-by: Bryce Mecum <[email protected]>
Co-authored-by: Joris Van den Bossche <[email protected]>
Signed-off-by: Bryce Mecum <[email protected]>
@jameslamb jameslamb mentioned this pull request Sep 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@h-vetinari h-vetinari h-vetinari approved these changes

@cpcloud cpcloud Awaiting requested review from cpcloud cpcloud is a code owner

@jakirkham jakirkham Awaiting requested review from jakirkham jakirkham is a code owner

@jreback jreback Awaiting requested review from jreback jreback is a code owner

@kou kou Awaiting requested review from kou kou is a code owner

@kszucs kszucs Awaiting requested review from kszucs kszucs is a code owner

@leifwalsh leifwalsh Awaiting requested review from leifwalsh leifwalsh is a code owner

@nealrichardson nealrichardson Awaiting requested review from nealrichardson nealrichardson is a code owner

@pcmoritz pcmoritz Awaiting requested review from pcmoritz pcmoritz is a code owner

@pearu pearu Awaiting requested review from pearu pearu is a code owner

@pitrou pitrou Awaiting requested review from pitrou pitrou is a code owner

@robertnishihara robertnishihara Awaiting requested review from robertnishihara robertnishihara is a code owner

@siddharthteotia siddharthteotia Awaiting requested review from siddharthteotia siddharthteotia is a code owner

@wesm wesm Awaiting requested review from wesm wesm is a code owner

@xhochy xhochy Awaiting requested review from xhochy xhochy is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@raulcd @h-vetinari