Closed ExpandingMan closed 1 year ago
Merging #30 (a87fa2f) into master (ab061e4) will decrease coverage by
0.43%
. The diff coverage is0.00%
.
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
- Coverage 55.11% 54.68% -0.44%
==========================================
Files 1 1
Lines 127 128 +1
==========================================
Hits 70 70
- Misses 57 58 +1
Impacted Files | Coverage Δ | |
---|---|---|
src/MLJXGBoostInterface.jl | 54.68% <0.00%> (-0.44%) |
:arrow_down: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Do you want to update the doc-strings, adding validate_parameters, to the list of hyper-parameters of each model?
Actually, it doesn't seem the parameters are explicitly documented. Actually this interface conforms with the default of validate_parameters
in the xgboost documentation whereas XGBoost.jl does not. I could add a blurb to all the doc strings mentioning it but that seems a bit much. Thoughts?
Actually, it doesn't seem the parameters are explicitly documented. Actually this interface conforms with the default > of validate_parameters in the xgboost documentation whereas XGBoost.jl does not.
Ok, right. In that case I don't think anything further is needed here.
Recently we changed XGBoost.jl so that it warns on invalid parameters by default (and fixed logging so that it goes through the Julia logger).
I really should have made sure this omitted most parameters when I overhauled it in the first place, but the way MLJ model objects work isn't exactly conducive to doing that.
This is admittedly a half-assed fix, but it will prevent users from unnecessarily getting bombarded with invalid parameter warnings.