-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
Core.py removing 'type: ignore' hints #9197
Core.py removing 'type: ignore' hints #9197
Conversation
python-package/xgboost/core.py
Outdated
pointers: ctypes.Array[ctypes.c_char_p] = (ctypes.c_char_p * len(data))() | ||
data_as_bytes = [bytes(d, "utf-8") for d in data] | ||
pointers[:] = data_as_bytes # type: ignore | ||
data_as_bytes: List[bytes] = [bytes(d, "utf-8") for d in data] | ||
pointers: ctypes.Array[ctypes.c_char_p] = (ctypes.c_char_p * len(data_as_bytes))(*data_as_bytes) | ||
return pointers | ||
raise TypeError() | ||
|
||
|
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.
This is out of my wheelhouse somewhat, but to me it looks like we're initialising an empty ctypes.Array (pointers
), then adding data_as_bytes
to it which was creating a mismatch between the List[bytes] and ctypes.Array[ctypes.c_char_p]. I've just added the data_as_bytes
into pointers
at initialisation, does this make sense?
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.
To be honest, I don't quite understand it either. Need to look it up. ;-)
Just committed the linting changes from https://github.com/dmlc/xgboost/actions/runs/5069030065/jobs/9121890991 |
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 good to me, thank you for removing the workarounds!
Apologies I've been away for a number of days. Thanks a lot for the review, merge and help finding something to work on @trivialfis 😄 |
As part of this issue, we want to remove some of the
type: ignore
hints to make the code more rigorous. This PR will aim to remove all thetype: ignore
hints in core.py