[ENH] skpro integration#200
Conversation
| "scikit-learn <1.8.0", | ||
| ] | ||
| sktime-integration = [ | ||
| "skpro", |
There was a problem hiding this comment.
Shouldn't we constrain the version here to avoid breaking changes in the future? (Also important for sktime and other deps.)
There was a problem hiding this comment.
this is only for the tests (imo), so no constraint is better - should skpro etc break anything, then we would see it in the tests, as a warning that something is no longer interoperable.
| _tags = { | ||
| "authors": "fkiraly", | ||
| "maintainers": "fkiraly", | ||
| "python_dependencies": "sktime", |
There was a problem hiding this comment.
This should be skpro, right?
|
|
||
| # default handling for cv | ||
| if isinstance(cv, int): | ||
| from sklearn.model_selection import KFold |
There was a problem hiding this comment.
Redundant imports. Should be refactored
| @@ -0,0 +1,314 @@ | |||
| """Experiment adapter for skpro corss-validation experiments.""" | |||
| self._scoring = scoring | ||
|
|
||
| # Set the sign of the scoring function (rely on sklearn scorer if present) | ||
| if self._scoring.get_tag("lower_is_better"): |
There was a problem hiding this comment.
I guess the following will happen here:
If "self._scoring = scoring"
then:
"AttributeError: 'str' object has no
attribute 'get_tag'"
Can you confirm?
There was a problem hiding this comment.
it needs to be an skpro metric, so should always have get_tag?
SimonBlanke
left a comment
There was a problem hiding this comment.
Some small fixed needed + Questions
|
(changes made) |
Closes #194, adds integration with
skpro:ProbaRegExperimentto benchmarkskproestimatorsProbaRegOptCVto tuneskproestimatorsAlso refactors the handling of default splitter coercion to a common module in
experiment.integrations._skl_cv.