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

added support for lightgbm booster #540

Merged
merged 8 commits into from
Aug 30, 2021
Merged

Conversation

marsupialtail
Copy link
Contributor

Issue #539 : added support for LightGBM booster

@ghost
Copy link

ghost commented Aug 20, 2021

CLA assistant check
All CLA requirements met.

@ksaur
Copy link
Collaborator

ksaur commented Aug 20, 2021

@marsupialtail Thanks so much for the PR! :) I think there are some lint things to check on in the CI/CD.

Can you please also add tests inside of test_lightgbm_converter.py to make sure that all lines of convert_lgbm_booster hit coverage? Thank you! And please let me know if you have questions; happy to help.

@marsupialtail
Copy link
Contributor Author

I added the tests based on the LGBRanker PR you referred. Not sure if that's what you are looking for.

@ksaur
Copy link
Collaborator

ksaur commented Aug 23, 2021

@marsupialtail I think there is another lint thing. (I tried to give permissions so the CI/CD would automatically run for you but it didn't seem to work.)

PR looks great otherwise! (cc: @interesaaat any comments? )

@ksaur ksaur linked an issue Aug 23, 2021 that may be closed by this pull request
@ksaur
Copy link
Collaborator

ksaur commented Aug 24, 2021

Ok I think I fixed it so you should be able to run the ci/cd without additional permissions. (Github must have changed some setting, because the subsequent run should have 'just ran')

There's some formatting issue, please take a look. Thanks!

@marsupialtail
Copy link
Contributor Author

Sorry but I absolutely hate lint. Not sure if I will have too much motivation to finish fixing the lint problems :-(

@codecov-commenter
Copy link

codecov-commenter commented Aug 29, 2021

Codecov Report

Merging #540 (6b41229) into main (4cd9c04) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #540      +/-   ##
==========================================
+ Coverage   90.62%   90.63%   +0.01%     
==========================================
  Files          78       78              
  Lines        4510     4516       +6     
  Branches      839      839              
==========================================
+ Hits         4087     4093       +6     
  Misses        237      237              
  Partials      186      186              
Flag Coverage Δ
unittests 90.47% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
hummingbird/ml/operator_converters/lightgbm.py 96.36% <100.00%> (+0.44%) ⬆️
hummingbird/ml/supported.py 92.98% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4cd9c04...6b41229. Read the comment docs.

@ksaur ksaur merged commit 946dcb0 into microsoft:main Aug 30, 2021
@ksaur
Copy link
Collaborator

ksaur commented Aug 30, 2021

Thanks for the contribution and for sticking with it @marsupialtail !

@marsupialtail marsupialtail deleted the lgbbooster branch August 30, 2021 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

loading lightgbm.basic.Booster
3 participants