FIX Extract estimator objects before aggregating dict of scores #17745
Conversation
|
You are super-efficient. You open a PR before that I found that the CI was broken :) |
|
We should probably have a test to trigger this usage. I am thinking that this is maybe the job of the |
I was looking to the failures in the CRON jobs and I realize that the documentation build was failing. Since I love keep things properly working, I propose the PR :) |
|
So maybe your solution is better then :) |
| if key.endswith(("time", "score")) | ||
| else [score[key] for score in scores] |
There was a problem hiding this comment.
I would prefer to dispatch based on the type:
return {
key: np.asarray([score[key] for score in scores])
if isinstance(scores[0][key], numbers.Number)
else [score[key] for score in scores]
for key in scores[0]
}and then update the docstring:
Aggregate a list of dicts to a dict of ndarray or list.
WTYD @glemaitre ?
There was a problem hiding this comment.
I made the change and merged so that PRs are not failing. We can have a followup PR if we want to go back to checking the key.
There was a problem hiding this comment.
Hmm... the only recent PR that was blocked was #17743
|
Thank you @alfaro96 for being so quick with the fix! |
|
Thanks @thomasjpfan and @glemaitre for taking care and finalizing this PR! |
…it-learn#17745) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
…it-learn#17745) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Reference Issues/PRs
Coming from #17332.
What does this implement/fix? Explain your changes.
This PR extracts the estimator objects for each cross-validation split before executing the
_aggregate_score_dictsincross_validationfunction to avoid that nested estimators are converted to arrays.Any other comments?
WDYT @thomasjpfan about this solution?
Another idea to solve this issue?