[dask] add tests on warnings, fix incorrect variable in log

[jameslamb]

While working on #3756 , I've been running mypy python-package/. This caught a bug in my refactoring from recent PRs (not sure which one).

python-package/lightgbm/dask.py:251: error: Name 'tree_learner' is not defined

This PR fixes that, and adds unit tests to cover the behavior under the two warnings in lightgbm.dask._train().

[StrikerRUS]

I've been running mypy python-package/.

Could you please setup mypy in lint task but allow it to fail (|| exit 0) so that everyone will be able to see logs and possibly prevent new issues to appear. We had the same strategy with cpplint. We had been checking logs manually but at some moment we simply removed || exit 0.

LightGBM/.ci/test.sh

Line 42 in 066720e

if [[ $TASK == "lint" ]]; then

[StrikerRUS]

Thanks! Just one proposal for even more informative name.

[StrikerRUS]

Just wonder: is it possible to access params via attributes like it can be done in sklearn-wrapper?

dask_regressor.tree_learner

[jameslamb]

maybe! I haven't tested. I didn't actually know the sklearn interface supported that.

But either way, I have a preference for get_params() here because it also gives us some confidence that this is working:

LightGBM/python-package/lightgbm/dask.py

Lines 419 to 420 in 59153b2

	self.set_params(**model.get_params())
	self._copy_extra_params(model, self)

[StrikerRUS]

Exactly set_params() should set attributes btw

LightGBM/python-package/lightgbm/sklearn.py

Lines 358 to 376 in b4b1b75

	def set_params(self, **params):
	"""Set the parameters of this estimator.
	Parameters
	----------
	**params
	Parameter names with their new values.
	Returns
	-------
	self : object
	Returns self.
	"""
	for key, value in params.items():
	setattr(self, key, value)
	if hasattr(self, '_' + key):
	setattr(self, '_' + key, value)
	self._other_params[key] = value
	return self

[jameslamb]

I've been running mypy python-package/.

Could you please setup mypy in lint task but allow it to fail (|| exit 0) so that everyone will be able to see logs and possibly prevent new issues to appear. We had the same strategy with cpplint. We had been checking logs manually but at some moment we simply removed || exit 0.

LightGBM/.ci/test.sh

Line 42 in 066720e

if [[ $TASK == "lint" ]]; then

sure! Sounds like an excellent good first issue idea for people to fix warnings.

[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.