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-package] fix mypy typing discrepancies in basic.py #5731

Closed
wants to merge 1 commit into from
Closed

[python-package] fix mypy typing discrepancies in basic.py #5731

wants to merge 1 commit into from

Conversation

IdoKendo
Copy link
Contributor

Contributes to #3867.
mypy currently raises the following errors in basic.py file:

python-package\lightgbm\basic.py:1127: error: Incompatible types in assignment (expression has type "_Pointer[c_int64]", variable has type "_Pointer[c_int32]")  [assignment]
python-package\lightgbm\basic.py:1132: error: Incompatible types in assignment (expression has type "_Pointer[c_double]", variable has type "_Pointer[c_float]")  [assignment]
python-package\lightgbm\basic.py:1444: error: Incompatible types in assignment (expression has type "c_void_p", variable has type "None")  [assignment]
python-package\lightgbm\basic.py:1448: error: Argument 1 to "byref" has incompatible type "None"; expected "_CData"  [arg-type]
python-package\lightgbm\basic.py:1499: error: Incompatible types in assignment (expression has type "c_void_p", variable has type "None")  [assignment]
python-package\lightgbm\basic.py:1510: error: Argument 1 to "byref" has incompatible type "None"; expected "_CData"  [arg-type]
python-package\lightgbm\basic.py:1595: error: Item "None" of "Optional[Dict[str, Any]]" has no attribute "get"  [union-attr]
python-package\lightgbm\basic.py:1689: error: Incompatible types in assignment (expression has type "c_void_p", variable has type "None")  [assignment]
python-package\lightgbm\basic.py:1694: error: Argument 1 to "byref" has incompatible type "None"; expected "_CData"  [arg-type]
python-package\lightgbm\basic.py:1827: error: Incompatible types in assignment (expression has type "c_void_p", variable has type "None")  [assignment]
python-package\lightgbm\basic.py:1842: error: Argument 1 to "byref" has incompatible type "None"; expected "_CData"  [arg-type]
python-package\lightgbm\basic.py:1857: error: Incompatible types in assignment (expression has type "Array[_Pointer[c_float]]", variable has type "Array[_Pointer[c_double]]")  [assignment]
python-package\lightgbm\basic.py:1883: error: Incompatible types in assignment (expression has type "c_void_p", variable has type "None")  [assignment]
python-package\lightgbm\basic.py:1887: error: Argument 1 to "c_int" has incompatible type "Optional[Any]"; expected "int"  [arg-type]
python-package\lightgbm\basic.py:1893: error: Argument 1 to "byref" has incompatible type "None"; expected "_CData"  [arg-type]
python-package\lightgbm\basic.py:1905: error: Incompatible types in assignment (expression has type "c_void_p", variable has type "None")  [assignment]
python-package\lightgbm\basic.py:1924: error: Argument 1 to "byref" has incompatible type "None"; expected "_CData"  [arg-type]
python-package\lightgbm\basic.py:1936: error: Incompatible types in assignment (expression has type "c_void_p", variable has type "None")  [assignment]
python-package\lightgbm\basic.py:1955: error: Argument 1 to "byref" has incompatible type "None"; expected "_CData"  [arg-type]
python-package\lightgbm\basic.py:2030: error: Incompatible types in assignment (expression has type "c_void_p", variable has type "None")  [assignment]
python-package\lightgbm\basic.py:2037: error: Argument 1 to "byref" has incompatible type "None"; expected "_CData"  [arg-type]
python-package\lightgbm\basic.py:2322: error: Value of type variable "SupportsRichComparisonT" of "sorted" cannot be "object"  [type-var]
python-package\lightgbm\basic.py:2565: error: Incompatible return value type (got "Union[List[Any], ndarray[Any, Any], Any, Any, None]", expected "Optional[ndarray[Any, 
Any]]")  [return-value]
python-package\lightgbm\basic.py:4256: error: Need type annotation for "values" (hint: "values: List[<type>] = ...")  [var-annotated]
python-package\lightgbm\basic.py:4326: error: No overload variant of "__setitem__" of "list" matches argument types "int", "ndarray[Any, dtype[floating[_64Bit]]]"  [call-overload]
python-package\lightgbm\basic.py:4326: note: Possible overload variants:
python-package\lightgbm\basic.py:4326: note:     def __setitem__(self, SupportsIndex, None, /) -> None
python-package\lightgbm\basic.py:4326: note:     def __setitem__(self, slice, Iterable[None], /) -> None
python-package\lightgbm\basic.py:4330: error: "None" has no attribute "ctypes"  [attr-defined]
python-package\lightgbm\basic.py:4336: error: Argument 1 to "len" has incompatible type "None"; expected "Sized"  [arg-type]
python-package\lightgbm\basic.py:4341: error: "None" has no attribute "size"  [attr-defined]
python-package\lightgbm\basic.py:4342: error: "None" has no attribute "reshape"  [attr-defined]
python-package\lightgbm\basic.py:4343: error: Incompatible return value type (got "Optional[Any]", expected "ndarray[Any, Any]")  [return-value]

These errors occur due to missing type annotations in the code and lack of None type validations.
This PR proposes fixing these errors by adding the missing annotations and None type checks.

Notes for Reviewers

This was tested by running mypy as documented in #3867.

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

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.

Thanks!

For some of these proposed changes, I'll wait to review them until the other in-progress mypy and type hinting PRs that are currently open have been approved and merged.

For the changes about pointers...I'm nervous about taking a runtime dependency on things from ctypes which are prefixed with _, like ctypes._CData. If those things are removed in future releases of ctypes, import lightgbm will fail. That risk is not acceptable for the benefit of slightly more specific type hints.

What do you think about the approach I've proposed in #5729, to use string literals for such things to avoid runtime import errors in that situation?

@jameslamb
Copy link
Collaborator

@IdoKendo are you still interested in pursuing this PR?

If so, please see me comment above about using string literals for hints like ctypes._Pointer, and please update this branch to latest master.

If not (or if you'd prefer to work on something else), just let me know and I'll continue addressing these particular mypy errors.

@IdoKendo
Copy link
Contributor Author

IdoKendo commented Mar 9, 2023

@IdoKendo are you still interested in pursuing this PR?

If so, please see me comment above about using string literals for hints like ctypes._Pointer, and please update this branch to latest master.

If not (or if you'd prefer to work on something else), just let me know and I'll continue addressing these particular mypy errors.

@jameslamb Unfortunately, not sure I have the capacity at this time, I'll close this PR so you can address it, and hopefully, soon I can get back to helping more.

@jameslamb
Copy link
Collaborator

ok no problem! Thanks so much for all your help so far, you've really helped re-invigorate this type annotation project that we'd been neglecting for a bit. Would love to have you come back and contribute any time.

@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 15, 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