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

[R] various R code maintenance #1964

Merged
merged 15 commits into from
Jan 21, 2017
Merged

Conversation

khotilov
Copy link
Member

This PR is to clear a number of long-standing and recently found annoyances and problems.

  • Most of functions must work with an xgb.Booster model when it has an invalid handle, e.g., when it was loaded as a regular R object. Also provide users with a function xgb.Booster.complete to restore a missing handle. Relevant: save the model under caret #1955, booster handle is invalid #1958
  • Storing the evaluation_log in an xgb.Booster should only depend on the presence of watchlist, but not on verbose value. I've got a number of complaints about this previous side-effect of verbose.
  • Reduce the amount of useless printouts from unit tests using this easy verbose=0 switch.
  • Windows 64 should still run all the tests.
  • Storing feature names of training data with xgb.Booster is useful for bookkeeping and for making further analysis of the model easier.
  • Co-occurrence computation was broken for quite awhile, and there was an agreement to get rid of it Variable Importance with co-occurence computation does not work #1092.
  • Documentation and tests updates.

I didn't add anything relevant to the new fast histogram updater yet.

@tqchen
Copy link
Member

tqchen commented Jan 16, 2017

@hetong007 can you help review this?

@tqchen tqchen requested a review from hetong007 January 16, 2017 07:12
@codecov-io
Copy link

codecov-io commented Jan 17, 2017

Current coverage is 30.73% (diff: 100%)

Merging #1964 into master will increase coverage by 0.03%

@@             master      #1964   diff @@
==========================================
  Files            72         72          
  Lines          5821       5821          
  Methods         885        885          
  Messages          0          0          
  Branches        580        580          
==========================================
+ Hits           1787       1789     +2   
+ Misses         3956       3955     -1   
+ Partials         78         77     -1   

Powered by Codecov. Last update a073a2c...8c1f22f

@hetong007
Copy link
Member

I have reviewed this PR, it seems good to me. Thank you @khotilov .

A meta question, seems new to me, is: How do I "approve" this PR? @tqchen

@tqchen
Copy link
Member

tqchen commented Jan 20, 2017

@tqchen tqchen merged commit 2b5b96d into dmlc:master Jan 21, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants