-
Notifications
You must be signed in to change notification settings - Fork 60
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
Support saving/loading of GBDT models #269
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #269 +/- ##
==========================================
+ Coverage 93.20% 93.25% +0.04%
==========================================
Files 115 115
Lines 5680 5719 +39
==========================================
+ Hits 5294 5333 +39
Misses 386 386 ☔ View full report in Codecov by Sentry. |
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.
Thanks! Can you please add tests for loading/saving? You can follow https://github.com/pyg-team/pytorch-frame/blob/master/test/utils/test_io.py#L14-L16
@weihua916 Add a test case for it. Is it correct? |
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.
Thanks! Left some comments to simplify and strengthen the tests.
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.
Looks mostly good! Final nit.
@weihua916 @yiweny Done. (Bad network, took some time to install mypy lol) |
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.
LGTM after resolving the comment. Let's merge after a pass from @weihua916
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.
Thanks!
for more information, see https://pre-commit.ci
… into gbdts/save-and-load
#242