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
Define metainfo for all DMatrix interfaces. #6601
Conversation
719dbb3
to
b21a3a4
Compare
Codecov Report
@@ Coverage Diff @@
## master #6601 +/- ##
==========================================
- Coverage 81.13% 80.08% -1.06%
==========================================
Files 13 13
Lines 3679 3675 -4
==========================================
- Hits 2985 2943 -42
- Misses 694 732 +38
Continue to review full report at Codecov.
|
70be2f4
to
1a2f548
Compare
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.
Please address my comment at #6591 (comment). You should choose between two alternatives:
- Break backward compatibility now. This might be reasonable choice if it's necessary to unity Dask and non-Dask interfaces; OR
- Don't break backward compatibility now; only add deprecation warning. For this alternative, all new parameters should only be added at the end of each function, not in the middle, to avoid breaking existing code using positional args.
My review below assumes that you've chosen Option 2. Let me know if you want to go with Option 1.
I will try to avoid breaking the compatibility. |
1a2f548
to
4884b97
Compare
@hcho3 I limited the breaking changes to |
This PR ensures all DMatrix types have a common interface. * Check for consistency between DMatrix types. * Add doc for bounds.
4884b97
to
85930fc
Compare
A small part extracted from #6591 to ease reviewing.