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

[ci] [python-package] fix mypy errors in sklearn.py #5886

Merged
merged 1 commit into from
May 16, 2023

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented May 12, 2023

Contributes to #3756.
Contributes to #3867.

Fixes the following errors from mypy.

python-package/lightgbm/sklearn.py:751: error: List item 0 has incompatible type "Union[str, Callable[[Optional[Any], Any], Tuple[str, float, bool]], Callable[[Optional[Any], Any], List[Tuple[str, float, bool]]], Callable[[Optional[Any], Any, Optional[Any]], Tuple[str, float, bool]], Callable[[Optional[Any], Any, Optional[Any]], List[Tuple[str, float, bool]]], Callable[[Optional[Any], Any, Optional[Any], Optional[Any]], Tuple[str, float, bool]], Callable[[Optional[Any], Any, Optional[Any], Optional[Any]], List[Tuple[str, float, bool]]], None]"; expected "Union[str, Callable[[Optional[Any], Any], Tuple[str, float, bool]], Callable[[Optional[Any], Any], List[Tuple[str, float, bool]]], Callable[[Optional[Any], Any, Optional[Any]], Tuple[str, float, bool]], Callable[[Optional[Any], Any, Optional[Any]], List[Tuple[str, float, bool]]], Callable[[Optional[Any], Any, Optional[Any], Optional[Any]], Tuple[str, float, bool]], Callable[[Optional[Any], Any, Optional[Any], Optional[Any]], List[Tuple[str, float, bool]]]]" [list-item]

python-package/lightgbm/sklearn.py:959: error: Incompatible return value type (got "Union[str, Union[Callable[[Optional[Any], Any], Tuple[Any, Any]], Callable[[Optional[Any], Any, Optional[Any]], Tuple[Any, Any]], Callable[[Optional[Any], Any, Optional[Any], Optional[Any]], Tuple[Any, Any]]], None]", expected "Union[str, Union[Callable[[Optional[Any], Any], Tuple[Any, Any]], Callable[[Optional[Any], Any, Optional[Any]], Tuple[Any, Any]], Callable[[Optional[Any], Any, Optional[Any], Optional[Any]], Tuple[Any, Any]]]]") [return-value]

@@ -956,7 +960,7 @@ def objective_(self) -> Union[str, _LGBM_ScikitCustomObjectiveFunction]:
""":obj:`str` or :obj:`callable`: The concrete objective used while fitting this model."""
if not self.__sklearn_is_fitted__():
raise LGBMNotFittedError('No objective found. Need to call fit beforehand.')
return self._objective
return self._objective # type: ignore[return-value]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the same as what's described in #5788 .

self._objective is initialized to None.

objective: Optional[Union[str, _LGBM_ScikitCustomObjectiveFunction]] = None,

And then overwritten with a string like "regression" or a custom objective function during training.

But mypy doesn't understand that it'll always be non-None if self.__sklearn_is_fitted__() is True (in other words, that it's not possible under normal operation for a user to ever call model.objective_ and get None back).

elif isinstance(eval_metric, list):
eval_metric_list = copy.deepcopy(eval_metric)
else:
eval_metric_list = [copy.deepcopy(eval_metric)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This refactoring adds an explicit case for eval_metric is None. mypy can't tell, I guess, that isinstance(x, list) would must mean something isn't None.

@jameslamb jameslamb marked this pull request as ready for review May 12, 2023 04:39
@jameslamb jameslamb merged commit 452370a into master May 16, 2023
@jameslamb jameslamb deleted the ci/mypy-sklearn branch May 16, 2023 02:38
@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