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

Add support for Python 3.12, drop pyppeteer dependency #625

Merged
merged 6 commits into from
Sep 10, 2024

Conversation

jameslamb
Copy link
Member

@jameslamb jameslamb commented Sep 4, 2024

Description

Contributes to rapidsai/build-planning#40

This PR adds support for Python 3.12.

Notes for Reviewers

This is part of ongoing work to add Python 3.12 support across RAPIDS.
It temporarily introduces a build/test matrix including Python 3.12, from rapidsai/shared-workflows#213.

A follow-up PR will revert back to pointing at the branch-24.10 branch of shared-workflows once all
RAPIDS repos have added Python 3.12 support.

Additional changes needed to add this support:

This will fail until all dependencies have been updates to Python 3.12

CI here is expected to fail until all of this project's upstream dependencies support Python 3.12.

This can be merged whenever all CI jobs are passing.

@jameslamb jameslamb added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Sep 4, 2024
@jameslamb jameslamb changed the title Add support for Python 3.12 WIP: Add support for Python 3.12 Sep 4, 2024
@@ -39,11 +39,11 @@ conda install ipykernel

# for stable rapids version
conda install -c rapidsai -c numba -c conda-forge -c nvidia \
cuxfilter=23.02 python=3.11 cudatoolkit=11.8
cuxfilter python=3.12 cudatoolkit=11.8
Copy link
Member Author

Choose a reason for hiding this comment

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

The Python version change pulled this into the diff, which made me realize the outdated cuxfilter=23.02 here. I definitely think that should be removed.

Should the cudatoolkit also be removed? I'd expect its dependencies like cuspatial and cudf to manage their own runtime dependencies on things from the CTK, and that separately installing cudatoolkit shouldn't be required.

Copy link
Contributor

@bdice bdice Sep 9, 2024

Choose a reason for hiding this comment

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

We should:

  • Drop -c numba from the line above
  • Replace this with something like what we have in cudf:
conda install -c rapidsai -c conda-forge -c nvidia \
    cuxfilter=24.10 python=3.12 cuda-version=12.5

Make sure update-version.sh has a replacement command for RAPIDS versions in notebooks/README.md.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok great, will push that shortly.

Looks like that's what this project does already do in its main README, so it's just this notebook README that needs to be updated.

cuxfilter/README.md

Lines 156 to 178 in ca63e9d

For nightly version `cuxfilter version == 24.10` :
```bash
# for CUDA 12.0
conda install -c rapidsai-nightly -c conda-forge -c nvidia \
cuxfilter=24.10 python=3.11 cuda-version=12.0
# for CUDA 11.8
conda install -c rapidsai-nightly -c conda-forge -c nvidia \
cuxfilter=24.10 python=3.11 cuda-version=11.8
```
For the stable version of `cuxfilter` :
```bash
# for CUDA 12.0
conda install -c rapidsai -c conda-forge -c nvidia \
cuxfilter python=3.11 cuda-version=12.0
# for CUDA 11.8
conda install -c rapidsai -c conda-forge -c nvidia \
cuxfilter python=3.11 cuda-version=11.8
```

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can reduce the content and only list it one place?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah even better

Copy link
Member Author

Choose a reason for hiding this comment

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

Just pushed a commit making these docs in notebooks/README.md point back to the installation steps in the top-level README.md.

@jameslamb
Copy link
Member Author

Now that cuspatial has added Python 3.12 support and has nightly packages available (rapidsai/cuspatial#1453), I've restarted CI here.

Will move this out of draft once everything is passing.

@@ -40,7 +40,6 @@ requirements:
- numpy >=1.23,<2.0a0
- packaging
- panel >=1.0
- pyppeteer >=0.2.6
Copy link
Member Author

Choose a reason for hiding this comment

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

This dependency on pyppeteer makes it impossible to install cuxfilter in a Python 3.12 environment.

I just pushed 7d17377 removing it.

It appears that the latest version of pyppeteer (v1.0.2, published 2.5+ years ago), puts an upper limit on websockets<11.0 and there are not any websockets conda packages available that both support Python 3.12 and meet that constraint.

The following packages are incompatible
└─ cuxfilter is installable with the potential options
   ├─ cuxfilter 24.10.00a21 would require
   │  └─ pyppeteer >=0.2.6  with the potential options
   │     ├─ pyppeteer 0.2.6 would require
   │     │  └─ websockets >=9.1,<10.0  with the potential options
   │     │     ├─ websockets [10.0|10.1|10.2|10.3|9.1] would require
   │     │     │  └─ python >=3.7,<3.8.0a0 , which can be installed;
   │     │     ├─ websockets [10.0|10.1|...|9.1] would require
   │     │     │  └─ python >=3.8,<3.9.0a0 , which can be installed;
   │     │     ├─ websockets [10.0|10.1|...|9.1] would require
   │     │     │  └─ python >=3.9,<3.10.0a0 , which can be installed;
   │     │     └─ websockets 9.1 would require
   │     │        └─ python >=3.6,<3.7.0a0 , which can be installed;
   │     └─ pyppeteer 1.0.2 would require
   │        └─ websockets >=10.0,<11.0  with the potential options
   │           ├─ websockets [10.0|10.1|10.2|10.3|10.4] would require
   │           │  └─ python >=3.10,<3.11.0a0 , which can be installed;
   │           ├─ websockets [10.0|10.1|10.2|10.3|9.1], which can be installed (as previously explained);
   │           ├─ websockets [10.0|10.1|...|9.1], which can be installed (as previously explained);
   │           ├─ websockets [10.0|10.1|...|9.1], which can be installed (as previously explained);
   │           └─ websockets 10.4 would require
   │              └─ python >=3.11,<3.12.0a0 , which can be installed;

(build link)

I think we can drop the pyppeteer dependency here!

@jameslamb jameslamb changed the title WIP: Add support for Python 3.12 WIP: Add support for Python 3.12, drop pyppeteer dependency Sep 9, 2024
@jameslamb jameslamb changed the title WIP: Add support for Python 3.12, drop pyppeteer dependency Add support for Python 3.12, drop pyppeteer dependency Sep 9, 2024
@jameslamb jameslamb marked this pull request as ready for review September 9, 2024 18:48
@jameslamb jameslamb requested review from a team as code owners September 9, 2024 18:48
@jameslamb jameslamb requested a review from bdice September 9, 2024 18:56
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

A few related cleanups -- I'll apply these and trigger a /merge.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@bdice
Copy link
Contributor

bdice commented Sep 10, 2024

/merge

@rapids-bot rapids-bot bot merged commit 53537fe into rapidsai:branch-24.10 Sep 10, 2024
31 checks passed
@jameslamb jameslamb deleted the python-3.12 branch September 10, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conda improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants