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

[python] Handle integer types more accurate in Python-to-C interface #4292

Merged
merged 1 commit into from
May 17, 2021

Conversation

StrikerRUS
Copy link
Collaborator

@StrikerRUS StrikerRUS commented May 15, 2021

The same as for R-package in #4291.

Match function signatures from https://github.com/microsoft/LightGBM/blob/master/include/LightGBM/c_api.h.

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

I went through this this morning. My review strategy was to pull up the Python package from this PR's branch and then the source of c_api.h in an editor, side-by-side.

image

I looked for each _LIB.something() call in the Python package and at the corresponding signature from the c_api header. All the signatures seem to match now, thanks very much!

@StrikerRUS
Copy link
Collaborator Author

@jameslamb Many thanks! My strategy was to search for ctypes. in basic.py file and track all calls of this module. I hope that two slightly different strategies give better chance to not overlook a mistake.

@jameslamb jameslamb merged commit 08c38ef into master May 17, 2021
@jameslamb jameslamb deleted the python_int branch May 17, 2021 20:42
@jameslamb jameslamb mentioned this pull request May 20, 2021
21 tasks
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants