-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[python-package] Allow to pass Arrow array as weights #6164
Conversation
@jameslamb I think you might need to request reviewers manually since they were removed earlier 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work! Thanks so much for the thorough tests, for bringing in pyarrow.compute
through the same way this project handles other conditional imports, and for switching over the tests to using that np_assert_array_equal()
function.
I left some minor comments, but overall I'm very happy with this addition to the Python API. Like in the other PRs, I'd like @guolinke or @shiyu1994 to review as well, particularly the C/C++ changes.
Don't worry about this lint
CI job complaining about the R package: https://github.com/microsoft/LightGBM/actions/runs/6791304427/job/18462554522?pr=6164
That's failing for reasons unrelated to this PR and I'll put up a PR soon to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me, thanks for the changes!
Let's wait for one more approval on the C/C++ changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
Motivation
This is part of a series of PRs towards #6022 and depends on #6163.
For a legible diff, see borchero/LightGBM@arrow-support-labels...arrow-support-weights.