-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[dask] remove unused private _client attribute #3904
Conversation
This reverts commit 98422be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is _client
not required? The other properties on scikit-learn estimators seem to all have an attribute like that with a single leading underscore:
LightGBM/python-package/lightgbm/sklearn.py
Lines 320 to 325 in c324e89
self._class_weight = None | |
self._class_map = None | |
self._n_features = None | |
self._n_features_in = None | |
self._classes = None | |
self._n_classes = None |
Yeah, but this is not required by sklearn. It is made for storing underlying value of a property (with training |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. I thought that #3883 (comment) referred to literally that one line you commented on, not the property completely.
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. |
Refer to #3883 (comment).