Typehint for some core API. #7348
Conversation
trivialfis
force-pushed
the
core-typehint
branch
from
3ebcb12
to
af2a0d8
Compare
Codecov Report
@@ Coverage Diff @@
## master #7348 +/- ##
==========================================
+ Coverage 83.70% 83.79% +0.08%
==========================================
Files 13 13
Lines 3890 3899 +9
==========================================
+ Hits 3256 3267 +11
+ Misses 634 632 -2
Continue to review full report at Codecov.
|
| @feature_names.setter | ||
| def feature_names(self, features: Optional[List[str]]) -> None: | ||
| self._set_feature_info(features, "feature_name") | ||
| @property |
There was a problem hiding this comment.
Mypy has issue with separated property getter/setter.
| @@ -2356,7 +2367,7 @@ def trees_to_dataframe(self, fmap=''): # pylint: disable=too-many-statements | |||
| node_ids = [] | |||
| fids = [] | |||
| splits = [] | |||
| categories = [] | |||
| categories: List[Optional[float]] = [] | |||
There was a problem hiding this comment.
Mostly for the Optional type that mypy can not infer.
| @@ -941,40 +939,26 @@ def apply( | |||
| iteration_range=iteration_range | |||
| ) | |||
|
|
|||
| def evals_result(self) -> TrainingCallback.EvalsLog: | |||
| def evals_result(self) -> Dict[str, Dict[str, List[float]]]: | |||
There was a problem hiding this comment.
EvalsLog contains CV result, while Sklearn interface uses only train.
| @@ -1354,48 +1338,6 @@ def predict_proba( | |||
| getattr(self, "n_classes_", None), class_probs, np.vstack | |||
| ) | |||
|
|
|||
| def evals_result(self) -> TrainingCallback.EvalsLog: | |||
There was a problem hiding this comment.
Removed as this function can be called by inheritance. We just have to unify the document.
| @@ -917,7 +915,7 @@ def apply( | |||
| Input features matrix. | |||
|
|
|||
| iteration_range : | |||
| See :py:meth:`xgboost.XGBRegressor.predict`. | |||
| See :py:meth:`predict`. | |||
There was a problem hiding this comment.
This way sphinx can automatically choose the correct method which belongs to current class.
| # Not improved | ||
| self.stopping_history[name][metric].append(score) | ||
| self.stopping_history[name][metric].append(score) # type: ignore |
There was a problem hiding this comment.
The type is Union[List[float], List[Tuple[float, float]], later is for cv function. A bit too tedious to cast it all the time.
| @@ -660,10 +661,7 @@ def _configure_fit( | |||
|
|
|||
| def _set_evaluation_result(self, evals_result: TrainingCallback.EvalsLog) -> None: | |||
| if evals_result: | |||
| for val in evals_result.items(): | |||
There was a problem hiding this comment.
Not needed, the very complicated transformation is just getting the same result as input.
Cross validation and low level primitives are not handled yet. Fix. Callback API. Fix build doc. Fix doc. Add allreduce type. More restricted type. Convert it first. Get the score working. lint. Cleanup. Cleanup the document. Use sequence instead of list or tuple. lint. Use sequence.
trivialfis
force-pushed
the
core-typehint
branch
from
1a065ee
to
2d754ea
Compare
Cross-validation and low-level primitives are not handled yet.
To have full coverage we need #7242 and a cleanly implemented of cv.
Sequenceinstead ofListfor public functions/methods.