rapidsai / docs

RAPIDS Documentation Site
https://docs.rapids.ai
37 stars 52 forks source link

install selector: Drop python 3.9 for nightlies, default to CUDA 12.5 for stable and nightly #534

Closed jameslamb closed 2 months ago

jameslamb commented 3 months ago

Contributes to https://github.com/rapidsai/build-planning/issues/88.

Proposes the following changes to the install selector:

Notes for Reviewers

How I tested this

Followed the instructions in the README. Ran locally and saw what I expected... Python 3.9 disabled when I tried selected Nightly.

Screenshot 2024-08-27 at 1 45 02 PM

But enabled when I selected Stable

Screenshot 2024-08-27 at 1 46 06 PM
netlify[bot] commented 3 months ago

Deploy Preview for docs-rapids-ai ready!

Built without sensitive environment variables

Name Link
Latest commit d093dbe5e828ad2d4609c1ba48965abdad7062a6
Latest deploy log https://app.netlify.com/sites/docs-rapids-ai/deploys/66ce1f75b020740007a391c2
Deploy Preview https://deploy-preview-534--docs-rapids-ai.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 3 months ago

Deploy Preview for docs-rapids-ai ready!

Built without sensitive environment variables

Name Link
Latest commit e6073f3169996ba1a67ee627dc7d6cf01a6aa3b3
Latest deploy log https://app.netlify.com/sites/docs-rapids-ai/deploys/66ce2845d80a980008a19c54
Deploy Preview https://deploy-preview-534--docs-rapids-ai.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

jakirkham commented 3 months ago

Thanks James! 🙏

This looks good

One thing that stood out is if Python 3.9 is selected for stable...

Screenshot 2024-08-27 at 11 54 12 AM

... and then nightly is picked, it still shows Python 3.9

Screenshot 2024-08-27 at 11 54 23 AM

Not sure if there is a way to fix this to move to a newer Python when selecting nightlies

Admittedly this might not be worth the trouble. Just wonder if it would cause confusion

jarmak-nv commented 3 months ago

... and then nightly is picked, it still shows Python 3.9

Usually I'd workaround this by adding a bit of logic in the releaseClickHandler saying if they click nightly and 3.9 is currently selected then update Python to 3.10 since it's in the same relative position (ie the lowest allowable version)

jameslamb commented 3 months ago

I can add something like that.

jameslamb commented 3 months ago

Ok, think I've handled that case: https://github.com/rapidsai/docs/pull/534/commits/e6073f3169996ba1a67ee627dc7d6cf01a6aa3b3

Try it out at https://deploy-preview-534--docs-rapids-ai.netlify.app/install

  1. select Stable (24.08) on the Release line
  2. select 3.9 on the Python line
  3. select Nightly (24.10a) on the Release line

You should see the following:

Tested the same thing with Docker, that looks like it's working as well.

I've written very little JavaScript, so I have no idea if what I'm doing is performant (and I know performance can matter for search rankings). Please do let me know if there are different data structures or patterns you think I should be using.

jarmak-nv commented 3 months ago

The code changes lgtm, I like that you made a more permanent implementation vs one that would need to be removed like I suggested.

My only q - the deploy preview isn't working for me and it doesn't look like netlify is reporting an error: Error code: PR_CONNECT_RESET_ERROR

https://www.netlifystatus.com/

Has anyone been able to check it? Just want to be sure there's not something causing an issue in the selector.

jakirkham commented 3 months ago

Yep I used the newest preview to test the installer selector to see how it behaved after the fix. It did the right thing of bumping to Python 3.10. Going back to stable it kept Python 3.10, which seems like a good approach

Am using macOS Safari and in statuses clicking netlify/docs-rapids-ai/deploy-preview, which appears to use: https://deploy-preview-534--docs-rapids-ai.netlify.app

FWIW how are you viewing the preview? Are you on a different OS or browser?

Curious just to make sure we don't have a JS bug in one context

jarmak-nv commented 2 months ago

Looks like it's working for me now - (I'm on mac firefox, but yeah no issue now.)

LGTM - thanks @jameslamb

jakirkham commented 2 months ago

Thanks James and Ben! 🙏