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

[ci] [dask] test lightgbm.dask on macOS #6677

Merged
merged 8 commits into from
Mar 2, 2025
Merged

[ci] [dask] test lightgbm.dask on macOS #6677

merged 8 commits into from
Mar 2, 2025
+11 −6

Conversation

jameslamb
Copy link
Collaborator

Contributes to #3782

Enables the Dask tests on macOS, based on this comment form @neNasko1 : #6569 (comment)

@jameslamb jameslamb mentioned this pull request Feb 12, 2025
Merged
jameslamb
jameslamb commented Feb 28, 2025
View reviewed changes
.vsts-ci.yml
@@ -254,6 +254,9 @@ jobs:
- script: |
git clean -d -f -x
displayName: 'Clean source directory'
# LGBM_SKIP_DASK_TESTS=true is set below only because running the tests under emulation is so slow...
# in theory, 'lightgbm.dask' should work without issue on aarch64 Linux systems.
# That could probably be removed as part of https://github.com/microsoft/LightGBM/issues/6788
Copy link
Collaborator Author

@jameslamb jameslamb Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

All of this LGBM_SKIP_DASK_TESTS machinery could be removed if #6843 is merged before this.

If this PR is merged first, then I'll remove that stuff in #6843

@jameslamb jameslamb changed the title WIP: [ci] [dask] test lightgbm.dask on macOS [ci] [dask] test lightgbm.dask on macOS Feb 28, 2025
@jameslamb
Copy link
Collaborator Author

This is now working! I think it's ready for review.

@jameslamb jameslamb marked this pull request as ready for review February 28, 2025 05:54
StrikerRUS
StrikerRUS approved these changes Mar 2, 2025
View reviewed changes
Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

Wow, great PR, thanks!
Very nice to see Dask working on macOS! 🎉

@StrikerRUS
Copy link
Collaborator

Just one question: why don't we test the latest Dask among the latest libraries?
https://github.com/microsoft/LightGBM/blob/master/.ci/test-python-latest.sh

@StrikerRUS StrikerRUS merged commit f7c641d into master Mar 2, 2025
50 checks passed
@StrikerRUS StrikerRUS deleted the ci/mac-dask-tests branch March 2, 2025 15:08
@StrikerRUS
Copy link
Collaborator

Also, maybe it's better to use feature label for this PR to help users discover info about Dask support on macOS? I bet very few people read Maintenance section of Release Notes 😄

@jameslamb
Copy link
Collaborator Author

Sure, we can switch this to feature. I was thinking maintenance because it is just adding testing (it was technically possible to use lightgbm.dask on macOS before), but I can understand how feature because this PR means this is now more fully "supported".

@jameslamb
Copy link
Collaborator Author

Just one question: why don't we test the latest Dask among the latest libraries? https://github.com/microsoft/LightGBM/blob/master/.ci/test-python-latest.sh

I think I did not add it there when setting that up because there weren't nightly versions published of dask / distributed ... so lots of other CI jobs already were capturing testing LightGBM against the latest releases of dask / distributed.

I'd support trying something like this in those jobs if you'd like to try it:

python -m pip install 'git+https://github.com/dask/dask.git@main'

Although some things to note:

@jameslamb
Copy link
Collaborator Author

oops sorry, I meant feature not featurevrequest, thank you for fixing that @StrikerRUS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StrikerRUS StrikerRUS StrikerRUS approved these changes

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

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

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

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

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

Successfully merging this pull request may close these issues.
2 participants
@jameslamb @StrikerRUS