add lightgbm.booster support by qh582 · Pull Request #270 · TeamHG-Memex/eli5

qh582 · 2018-05-29T03:55:41Z

mainly followed the logic of xgboost.booster support

add lightgbm.booster support

lgb2.1.0 to lgb 2.1.1

update to lgb2.1.1

codecov-io · 2018-05-29T04:49:49Z

Codecov Report

Merging #270 into master will decrease coverage by <.01%.
The diff coverage is 97.91%.

@@            Coverage Diff             @@
##           master     #270      +/-   ##
==========================================
- Coverage   97.22%   97.22%   -0.01%     
==========================================
  Files          44       44              
  Lines        2815     2845      +30     
  Branches      536      545       +9     
==========================================
+ Hits         2737     2766      +29     
  Misses         41       41              
- Partials       37       38       +1

Impacted Files	Coverage Δ
eli5/lightgbm.py	`95.23% <97.91%> (+0.36%)`	⬆️

lopuhin · 2018-05-29T07:23:03Z

eli5/lightgbm.py


-
+def _check_booster_args(lgb, is_regression=None):
+    # type: (Any, bool) -> Tuple[Booster, bool]


I hope something like # type: (Any, bool) -> Tuple[lightgbm.Booster, bool] and adding Tuple and Any to typing imports at the top should fix the build (https://travis-ci.org/TeamHG-Memex/eli5/jobs/385021206#L764-L766). To check locally, you can try installing mypy==0.550 and running mypy --check-untyped-defs eli5

lopuhin · 2018-05-29T07:26:31Z

Thanks for the PR @qh582 , at first sight it looks good 👍
I left a comment on how to fix the build. It would be also great if you could add tests for lightgbm.booster support, so that we don't break it in the future.

qh582 · 2018-05-30T03:36:37Z

@lopuhin Thanks for review and suggestion.

solved it.

I am doing the explain_prediction for regression part test, and I got 'y=y' in stead of 'y' as shown in the screenshot:

This seems ok for me, but I found eli5 using assert '<b>y</b>' in strip_blanks(expl_html) in tests.test_sklearn_explain_prediction.assert_trained_linear_regression_explained which point that it shoud be 'y' in my case. I am wondering if I missed the point here.

add test

add explain_prediction test for regression booster

fix bug of using multiclass booster to fit binary case

qh582 · 2018-05-31T05:19:47Z

@lopuhin could you please check it?

lopuhin

Looks great, thanks @qh582 ! I'm +1 to merge this, @kmike do you want to have a look?

adding test done

kmike · 2018-05-31T11:03:01Z

@qh582 looks good! Would you mind updating a few notes in docs, mentioning this new feature?

* Update overview.rst * Update README.rst * Update lightgbm.rst

qh582 · 2018-05-31T12:17:05Z

@kmike I think this is ready, please check :)

kmike · 2018-05-31T12:28:31Z

docs/source/libraries/lightgbm.rst

+  ("objective" starts with "reg")
+  and False for a classification problem.
+  If not set, regression is assumed for a single target estimator
+  and proba will not be shown unless the ``target_names`` is defined as a list with length of two.


This seems to contradict a note above (line 25, "target_names is ignored") - does the note about target_names apply only to classifier/regressor, but not for Booster?

for a single target estimater, booster cannot recognize wheather it is a regression problem or not. We assume it is regression in default, unless users set it as a classification problem by assigning 'target names' input [0,1] etc. Only in this case 'target names' is used. Should I remove " target names is ignored" to prevent confusion？

target names is still ignored for classifier/regressor, right? I think it makes sense to clarify it - just say that it is ignored for LGBMClassifier / LGBMRegressor, but used for lightgbm.Booster.

"is defined as a list with length of two" - sohuld it be 2 elements, or 2+ is also supported?

Well, for single target booster, only 1+ elements target_name point booster to classification. Otherwise, this booster is regression . there is risk user assign wrong number of elements like 3, but not sure how eli5 will raise error. 2+ should not support here.

kmike · 2018-05-31T12:29:32Z

docs/source/libraries/lightgbm.rst

-and :func:`eli5.explain_prediction` for ``lightgbm.LGBMClassifer``
-and ``lightgbm.LGBMRegressor`` estimators.
+and :func:`eli5.explain_prediction` for ``lightgbm.LGBMClassifer``, ``lightgbm.LGBMRegressor`` and ``lightgbm.Booster``  estimators. It is tested against LightGBM
+master branch.


"It is tested against LightGBM master branch." is removed in 63e9918, sorry for the confusion. It shouldn't be re-added.

got it, I will remove that line in next commit

qh582 · 2018-05-31T15:10:28Z

@kmike Hope the new commit help. My previous piece of code is not strict enough.

WestXu · 2018-08-08T06:12:05Z

It's been two month, could someone tell me why this merge hasn't been closed already? I really need this.

lopuhin · 2018-11-14T17:59:46Z

I think PR is in great shape and we just need to check it again and merge it, hope to find time for it soonish, sorry for delay.

kmike · 2018-11-18T09:49:53Z

docs/source/libraries/lightgbm.rst


 :func:`eli5.explain_weights` uses feature importances. Additional
-arguments for LGBMClassifier and LGBMClassifier:
+arguments for LGBMClassifier , LGBMClassifier and lightgbm.Booster:


a nitpick: extra whitespace before comma

kmike · 2018-11-18T09:50:30Z

docs/source/libraries/lightgbm.rst

    :ref:`library-xgboost`.

-``target_names`` and ``target`` arguments are ignored.
+``target_names`` arguement is ignored for ``lightgbm.LGBMClassifer`` / ``lightgbm.LGBMRegressor``, but used for ``lightgbm.Booster``. ``target`` argument is ignored.


Suggested change

``target_names`` arguement is ignored for ``lightgbm.LGBMClassifer`` / ``lightgbm.LGBMRegressor``, but used for ``lightgbm.Booster``. ``target`` argument is ignored.

``target_names`` argument is ignored for ``lightgbm.LGBMClassifer`` / ``lightgbm.LGBMRegressor``, but used for ``lightgbm.Booster``. ``targets`` argument is ignored.

kmike · 2018-11-18T09:50:42Z

docs/source/libraries/lightgbm.rst


 :func:`eli5.explain_weights` uses feature importances. Additional
-arguments for LGBMClassifier and LGBMClassifier:
+arguments for LGBMClassifier , LGBMClassifier and lightgbm.Booster:


Suggested change

arguments for LGBMClassifier , LGBMClassifier and lightgbm.Booster:

arguments for LGBMClassifier, LGBMClassifier and lightgbm.Booster:

kmike · 2018-11-18T09:51:12Z

docs/source/libraries/lightgbm.rst

+``target_names`` arguement is ignored for ``lightgbm.LGBMClassifer`` / ``lightgbm.LGBMRegressor``, but used for ``lightgbm.Booster``. ``target`` argument is ignored.

 .. note::
    Top-level :func:`eli5.explain_weights` calls are dispatched


I think lightgbm.Booster should be mentioned here as well.

kmike · 2018-11-18T09:52:55Z

docs/source/libraries/lightgbm.rst


+``lightgbm.Booster`` estimator accepts one more optional argument:
+
+* ``is_regression`` - True if solving a regression problem


Is this parameter supported? It is not an argument of explain_prediction_lightgbm.

kmike · 2018-11-18T09:53:10Z

docs/source/overview.rst


 * :ref:`library-lightgbm` - show feature importances and explain predictions
-  of LGBMClassifier and LGBMRegressor.
+  of LGBMClassifier , LGBMRegressor and lightgbm.Booster.


Suggested change

of LGBMClassifier , LGBMRegressor and lightgbm.Booster.

of LGBMClassifier, LGBMRegressor and lightgbm.Booster.

kmike · 2018-11-18T09:56:16Z

eli5/lightgbm.py

-    ``target_names`` and ``targets`` parameters are ignored.
+    ``target_names`` arguement is ignored for ``lightgbm.LGBMClassifer`` / ``lightgbm.LGBMRegressor``, 
+    but used for ``lightgbm.Booster``. 
+    ``target`` argument is ignored.


Suggested change

``target`` argument is ignored.

``targets`` argument is ignored.

Dronablo · 2020-04-11T19:21:33Z

Any plans about merging it, @lopuhin?

lopuhin · 2021-02-10T07:55:04Z

Thanks you! This were merged in eli5-org/eli5#7 and released to PyPI with v0.11

qh582 added 4 commits May 29, 2018 11:34

add lightgbm.booster support

d369977

mainly followed the logic of xgboost.booster support

Merge pull request #1 from qh582/qh582-lightgbm-booster

cc5b7a4

add lightgbm.booster support

fix bug

9c2bb8e

lgb2.1.0 to lgb 2.1.1

bug fix

9c31a99

update to lgb2.1.1

lopuhin reviewed May 29, 2018

View reviewed changes

qh582 added 3 commits May 29, 2018 19:20

fix build for typing

a042650

add _check_booster_args test

72a50a2

add explain_weights test for booster

bb19a4f

qh582 added 9 commits May 30, 2018 11:54

Merge pull request #2 from qh582/qh582-lightgbm-booster

4bf2f6d

add test

correct is_regression

ef991cf

add explain_prediction test for regression booster

45379f5

Merge pull request #3 from qh582/qh582-test

a35e5c8

add explain_prediction test for regression booster

add booster multiclass test

0bbf3e2

fix booster binary case

fa1eccd

add test_explain_prediction_booster_binary

f053399

remove redundant code

87438d4

fix bug of using multiclass booster to fit binary case

add _lgb_n_targets test

3c213e9

lopuhin approved these changes May 31, 2018

View reviewed changes

Merge pull request #4 from qh582/qh582-lightgbm-booster

866015a

adding test done

qh582 added 3 commits May 31, 2018 19:56

Update docs

802e543

* Update overview.rst * Update README.rst * Update lightgbm.rst

update docs

5155979

* Update overview.rst * Update README.rst * Update lightgbm.rst

Merge branch 'master' into qh582-lightgbm-booster

6f1ee40

kmike reviewed May 31, 2018

View reviewed changes

update booster is_regression

0a488e3

Merge branch 'master' into qh582-lightgbm-booster

19e9bf3

lopuhin mentioned this pull request Nov 14, 2018

no support for LGBMRegressor #280

Closed

kmike reviewed Nov 18, 2018

View reviewed changes

kmike mentioned this pull request Nov 18, 2018

DOC small fixes to xgboost docs #285

Merged

lopuhin mentioned this pull request Jan 23, 2021

Add lightgbm.booster support eli5-org/eli5#7

Merged



		def _check_booster_args(lgb, is_regression=None):
		# type: (Any, bool) -> Tuple[Booster, bool]

	``target_names`` arguement is ignored for ``lightgbm.LGBMClassifer`` / ``lightgbm.LGBMRegressor``, but used for ``lightgbm.Booster``. ``target`` argument is ignored.
	``target_names`` argument is ignored for ``lightgbm.LGBMClassifer`` / ``lightgbm.LGBMRegressor``, but used for ``lightgbm.Booster``. ``targets`` argument is ignored.

	arguments for LGBMClassifier , LGBMClassifier and lightgbm.Booster:
	arguments for LGBMClassifier, LGBMClassifier and lightgbm.Booster:


		``lightgbm.Booster`` estimator accepts one more optional argument:

		* ``is_regression`` - True if solving a regression problem

	of LGBMClassifier , LGBMRegressor and lightgbm.Booster.
	of LGBMClassifier, LGBMRegressor and lightgbm.Booster.

	``target`` argument is ignored.
	``targets`` argument is ignored.

Conversation

qh582 commented May 29, 2018

Uh oh!

codecov-io commented May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lopuhin May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lopuhin commented May 29, 2018

Uh oh!

qh582 commented May 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qh582 commented May 31, 2018

Uh oh!

lopuhin left a comment

Choose a reason for hiding this comment

Uh oh!

kmike commented May 31, 2018

Uh oh!

qh582 commented May 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qh582 May 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qh582 commented May 31, 2018

Uh oh!

WestXu commented Aug 8, 2018

Uh oh!

lopuhin commented Nov 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmike Nov 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dronablo commented Apr 11, 2020

Uh oh!

lopuhin commented Feb 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Comments

codecov-io commented May 29, 2018 •

edited

Loading

lopuhin May 29, 2018 •

edited

Loading

qh582 commented May 30, 2018 •

edited

Loading

qh582 May 31, 2018 •

edited

Loading

kmike Nov 18, 2018 •

edited

Loading