Make some functions from Dask module class methods

[StrikerRUS]

It will be good to have _train() and _predict() functions (at least them, very possibly some other functions can be made methods) in a form of _LGBMModel's methods. It will improve encapsulation in Dask module.

LightGBM/python-package/lightgbm/dask.py

Line 179 in ac706e1

def _train(client, data, label, params, model_factory, sample_weight=None, group=None, **kwargs):

LightGBM/python-package/lightgbm/dask.py

Line 337 in ac706e1

def _predict(model, data, raw_score=False, pred_proba=False, pred_leaf=False, pred_contrib=False,

#3515 (comment)

[ShrillShrestha]

Hi @StrikerRUS, I am a beginner and learning to contribute in open source. I am not sure if I understand the whole issue, but it seems interesting. If you can guide me, I think I can resolve it.

Do I add these:

LightGBM/python-package/lightgbm/dask.py

Line 179 in b8fc476

def _train(client, data, label, params, model_factory, sample_weight=None, group=None, **kwargs):

and

LightGBM/python-package/lightgbm/dask.py

Line 337 in b8fc476

def _predict(model, data, raw_score=False, pred_proba=False, pred_leaf=False, pred_contrib=False,

inside the _LGBMModel class and change

LightGBM/python-package/lightgbm/dask.py

Line 395 in b8fc476

model = _train(

to model = self._train(...)
and return statements like

LightGBM/python-package/lightgbm/dask.py

Line 443 in b8fc476

return _predict(

to return super()._predict(...)

[jameslamb]

Hi @ShrillShrestha , thanks for the offer!

But @StrikerRUS I disagree with the label "good first issue" on this one. This will involve some design discussions that I think should be done by maintainers. It's not as simple as just moving the code around, because the decision here might be based on a discussion (that we have not had yet) about whether the Dask module should support a non-scikit, functional API similar to

LightGBM/python-package/lightgbm/engine.py

Line 15 in ac706e1

def train(params, train_set, num_boost_round=100,

. Let's please not have that conversation here by the way.

@ShrillShrestha all that said...we would love you help on one other issues. Could you try #3833?

[StrikerRUS]

@jameslamb

the decision here might be based on a discussion (that we have not had yet) about whether the Dask module should support a non-scikit, functional API similar to

Makes sense! I was not aware of plans to support functions from standard API train and cv.

Feel free to close this issue because I guess code refactoring indeed will be tightened to decisions we will make.

[jameslamb]

Thanks! Yes sorry, I have a lot of documentation to catch up on.

Added feature requests for train() (#3846) and cv() (#3847).

I'll close this issue, thanks.

[github-actions]

This issue 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.