Skip to content

[BUG] - bugfix, GridSearchCV scoring=None defaults to CRPS#1010

Open
MaazAli8460 wants to merge 2 commits intosktime:mainfrom
MaazAli8460:fix-issue-1009
Open

[BUG] - bugfix, GridSearchCV scoring=None defaults to CRPS#1010
MaazAli8460 wants to merge 2 commits intosktime:mainfrom
MaazAli8460:fix-issue-1009

Conversation

@MaazAli8460
Copy link
Copy Markdown

If scoring= None, set it to CRPS().

Reference Issues/PRs

Fixes #1009.

What does this implement/fix? Explain your changes.

If scoring= None, set it to CRPS(). in _tuning.py

Does your contribution introduce a new dependency? If yes, which one?

No.

What should a reviewer concentrate their feedback on?

Did you add any tests for the change?

No. I don't think a separate test is necessary here.

PR checklist

For all contributions
  • I've added myself to the list of contributors with any new badges I've earned :-)
    How to: add yourself to the all-contributors file in the skpro root directory (not the CONTRIBUTORS.md). Common badges: code - fixing a bug, or adding code logic. doc - writing or improving documentation or docstrings. bug - reporting or diagnosing a bug (get this plus code if you also fixed the bug in the PR).maintenance - CI, test framework, release.
    See here for full badge reference
  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG]. [BUG] - bugfix, [MNT] - CI, test framework, [ENH] - adding or improving code, [DOC] - writing or improving documentation or docstrings.
For new estimators
  • I've added the estimator to the API reference - in docs/source/api_reference/taskname.rst, follow the pattern.
  • I've added one or more illustrative usage examples to the docstring, in a pydocstyle compliant Examples section.
  • If the estimator relies on a soft dependency, I've set the python_dependencies tag and ensured
    dependency isolation, see the estimator dependencies guide.

If scoring= None, set it to CRPS().
Copilot AI review requested due to automatic review settings March 30, 2026 06:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes GridSearchCV crashing when scoring=None by applying the documented default metric (CRPS) during fitting.

Changes:

  • Import CRPS into skpro.model_selection._tuning.
  • In BaseGridSearch._fit, replace scoring=None with CRPS() before using scoring.name/scoring.get_tag.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 110 to 114
# scoring = check_scoring(self.scoring, obj=self)
scoring = self.scoring
if scoring is None:
scoring = CRPS()
scoring_name = f"test_{scoring.name}"
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a regression test to cover the scoring=None path (issue #1009). Without a test, this bug is likely to reappear since BaseGridSearch._fit relies on scoring.name/get_tag and will crash if scoring is ever left as None again. A minimal test can instantiate GridSearchCV with scoring=None, run fit, and assert that cv_results_ contains the expected CRPS-derived score column (e.g., mean_test_CRPS) and that no AttributeError is raised.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly does this fix?

If it fixes a bug, please describe it - possibly in an issue, if it is longer - and also add a test for the bug being fixed.

@MaazAli8460
Copy link
Copy Markdown
Author

MaazAli8460 commented Apr 1, 2026

@fkiraly Thanks for the review.
I have created an issue for this. Fixes #1009. It is also referenced here.
This PR fixes a bug that caused GridSearchCV to crash when scoring=None.
The change resolves scoring=None to CRPS() before scoring_name, matching the documented behavior.
Before the fix, BaseGridSearch._fit tried to read scoring.name while scoring was still None, causing: AttributeError: 'NoneType' object has no attribute 'name'.

Test Code:

import pandas as pd
from sklearn.model_selection import KFold

from skpro.model_selection import GridSearchCV
from skpro.regression.dummy import DummyProbaRegressor

X = pd.DataFrame({"x": [1, 2, 3, 4]})
y = pd.DataFrame({"y": [1.0, 2.0, 3.0, 4.0]})

gscv = GridSearchCV(
    estimator=DummyProbaRegressor(),
    cv=KFold(n_splits=2),
    param_grid={"strategy": ["empirical"]},
)

gscv.fit(X, y)
print(gscv.cv_results_)

In skpro\model_selection\ _tuning.py
BaseGridSearch._fit()
Before:

# scoring = check_scoring(self.scoring, obj=self)
        scoring = self.scoring
        scoring_name = f"test_{scoring.name}"

Raises error
AttributeError: 'NoneType' object has no attribute 'name'

After fix

# scoring = check_scoring(self.scoring, obj=self)
        scoring = self.scoring
        if scoring is None:
            scoring = CRPS()
        scoring_name = f"test_{scoring.name}"

On same test code

   mean_test_CRPS  mean_fit_time                     params  rank_test_CRPS
0            1.75       0.008404  {'strategy': 'empirical'}             1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] GridSearchCV scoring=None gives AttributeError

3 participants