[python-package] fix type hints on ctypes array converters

[jameslamb]

Contributes to #3756.
Contributes to #3867.

mypy currently raises the following errors.

python-package/lightgbm/basic.py:258: error: Function "ctypes.POINTER" is not valid as a type
python-package/lightgbm/basic.py:258: note: Perhaps you need "Callable[...]" or a callback protocol?
python-package/lightgbm/basic.py:266: error: Function "ctypes.POINTER" is not valid as a type
python-package/lightgbm/basic.py:266: note: Perhaps you need "Callable[...]" or a callback protocol?
python-package/lightgbm/basic.py:274: error: Function "ctypes.POINTER" is not valid as a type
python-package/lightgbm/basic.py:274: note: Perhaps you need "Callable[...]" or a callback protocol?
python-package/lightgbm/basic.py:282: error: Function "ctypes.POINTER" is not valid as a type
python-package/lightgbm/basic.py:282: note: Perhaps you need "Callable[...]" or a callback protocol?

These are correctly noting that ctypes.POINTER isn't a type...it's a function that returns a type. From https://docs.python.org/3/library/ctypes.html#ctypes.pointer

[ctypes.pointer()] creates a new pointer instance, pointing to obj. The returned object is of the type POINTER(type(obj))

You can see this with the following example code.

import ctypes

x = ctypes.c_float(1.1)
type(x)
# <class 'ctypes.c_float'>

ptr_type = ctypes.POINTER(type(x))
print(ptr_type)
# <class '__main__.LP_c_float'>

callable(ptr_type)
# True

isinstance(ctypes.pointer(x), ctypes.POINTER(ctypes.c_float))
# True

Since the functions mypy is complaining about all take in an object, check for a specific type with isinstance(), and raises an exception if the passed-in object doesn't have that type, I think it's appropriate to replace those ctypes.POINTER type hints with just Any.

Notes for Reviewers

I tested this by running mypy as documented in #3867.

mypy \
    --exclude='python-package/compile/|python-package/build' \
    --ignore-missing-imports \
    python-package/

[jmoralez]

I think we can instead use ctypes.pointer[ctypes.c_int32] for example. Reference: python/mypy#7540 (comment)

[jameslamb]

@jmoralez hmmm, that doesn't look right to me.

docker run \
    --rm \
    --entrypoint="" \
    -it python:3.8-slim \
    python -c "import ctypes; print(ctypes.POINTER[ctypes.c_int32])"

Traceback (most recent call last):
File "", line 1, in
TypeError: 'builtin_function_or_method' object is not subscriptable

I think that maybe the comment you found was someone getting lucky (or, I guess, unlucky) with the official type hints for ctypes being inaccurate.

See this from last month: python/typeshed#8351 (comment)

[jmoralez]

Ah I see, in the stub the lowercase pointer is a class but in ctypes it isn't haha. I think it's ok to go with Any.

[StrikerRUS]

Should we switch to ctypes._Pointer with a new mypy release in the future according to python/typeshed#8446 (comment)?

[jameslamb]

@StrikerRUS I'm nervous about relying on something preceded with a _, since I interpret that as being a private implementation detail that could change.

I want LightGBM's Python code to get the benefits of type-hinting, but I'm worried about the risk of user code being broken because of a change in typeshed.

[StrikerRUS]

Just for reference:

Especially given that ctypes._Pointer is actually documented -- it seems like it's not really meant to be a "private" class.
python/typeshed#8446 (comment)

However, I'm OK with Any.

[jameslamb]

Thanks for pointing out that comment.

Even with that, I'm just nervous about the amount of change happening around this topic (indicated by the issues, you, @jmoralez , and I have linked to) and don't think the benefit of more accurate hints for these small utility functions is worth taking on the risk of incompatibilities of lightgbm with different versions of ctypes.

[github-actions]

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.