NotificationsYou must be signed in to change notification settings
Fork26k
Star62.7k

FIX: Replace y_pred with y_score in DetCurveDisplay (issue: 31761)#31764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

luiser1401 wants to merge11 commits intoscikit-learn:main

base:main

Choose a base branch

fromluiser1401:solve-issue-luis

Open

FIX: Replace y_pred with y_score in DetCurveDisplay (issue: 31761)#31764

luiser1401 wants to merge11 commits intoscikit-learn:mainfromluiser1401:solve-issue-luis

+173 −85

Conversation

Copy link

luiser1401 commentedJul 15, 2025

This PR replaces y_pred with y_score in the DetCurveDisplay class. This change was made to follow the pattern in RocCurveDisplay.

Fixes:#31761

replace y_pred with y_score

85239d0

github-actionsbot added module:metrics Documentation labels

Jul 15, 2025

luiser1401 changed the title~~DOC: Replace y_pred with y_score in DetCurveDisplay (issue: 31761)~~FIX: Replace y_pred with y_score in DetCurveDisplay (issue: 31761)

Jul 15, 2025

Copy link

github-actionsbot commentedJul 15, 2025•
edited
Loading

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit:d98568f. Link to the linter CI:here}

luiser1401 added5 commits

July 15, 2025 20:54

update tests

80fec6d

extend tests

2ab1387

add changelog

c64e675

final update to test

3fafa6e

change y_pred to y_score in precision_recall_curve.py

291a1b6

luiser1401 mentioned this pull request

Jul 16, 2025

y_pred changed to y_true in RocCurveDisplay.from_predictions, but not in DetCurveDisplay.from_predictions#31761

Open

lucyleeow reviewed

Jul 17, 2025

View reviewed changes

Copy link

Member

lucyleeow left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Thanks@luiser1401 , just some suggestions.

CI failure is unrelated (we're working on fixing).

sklearn/metrics/_plot/det_curve.py Outdated

Comment on lines 304 to 319

		ify_scoreisnotNoneandnot (
		isinstance(y_pred,str)andy_pred=="deprecated"
		):
		raiseValueError(
		"`y_pred` and `y_score` cannot be both specified. Please use `y_score`"
		" only as `y_pred` is deprecated in 1.7 and will be removed in 1.9."
		)
		ifnot (isinstance(y_pred,str)andy_pred=="deprecated"):
		warnings.warn(
		(
		"y_pred is deprecated in 1.7 and will be removed in 1.9. "
		"Please use `y_score` instead."
		),
		FutureWarning,
		)
		y_score=y_pred

Copy link

Member

lucyleeowJul 17, 2025

Choose a reason for hiding this comment

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

Let's refactor this out tosklearn/utils/_plotting.py and it can be used for precision recall, ROC and DET displays

doc/whats_new/upcoming_changes/sklearn.metrics/31764.fix.rst Outdated

Comment on lines 1 to 5

		- :func:`metrics._plot.det_curve.DetCurveDisplay.from_predictions` y_pred parameter is deprecated and will be removed
		in v1.9. It has been replaced by y_score.
		- :func:`metrics._plot.precision_recall_curve.PrecisionRecallDisplay.from_predictions` y_pred parameter is deprecated
		and will be removed in v1.9. It has been replaced by y_score.
		By :user:`Luis <luiser1401>`

Copy link

Member

lucyleeowJul 17, 2025

Choose a reason for hiding this comment

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

What about:

Suggested change

	-:func:`metrics._plot.det_curve.DetCurveDisplay.from_predictions` y_pred parameter is deprecated and will be removed
	in v1.9. It has been replaced by y_score.
	-:func:`metrics._plot.precision_recall_curve.PrecisionRecallDisplay.from_predictions` y_pred parameter is deprecated
	and will be removed in v1.9. It has been replaced by y_score.
	By:user:`Luis <luiser1401>`
	- `y_pred` is deprecated in favour of `y_score` in:func:`metrics.DetCurveDisplay.from_predictions`
	and:func:`metrics.PrecisionRecallDisplay.from_predictions`. Both will be removed
	in v1.10.
	By:user:`Luis <luiser1401>`

Can't tell if lines are under 88 characters but please check this.
I think we're at v1.8 so removal will in in 2 versions, 1.10

Copy link

Author

luiser1401Jul 17, 2025

Choose a reason for hiding this comment

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

Will check line lenghts and structure the log how you suggested, however the last release is1.7 on Jun 6th

sklearn/metrics/_plot/tests/test_det_curve_display.py Outdated

Comment on lines 118 to 131

		def test_y_score_and_y_pred_specified_error():
		"""Check that an error is raised when both y_score and y_pred are specified."""
		y_true = np.array([0, 1, 1, 0])
		y_score = np.array([0.1, 0.4, 0.35, 0.8])
		y_pred = np.array([0.2, 0.3, 0.5, 0.1])

		with pytest.raises(
		ValueError, match="`y_pred` and `y_score` cannot be both specified"
		):
		DetCurveDisplay.from_predictions(y_true, y_score=y_score, y_pred=y_pred)


		# TODO(1.9): remove
		def test_y_pred_deprecation_warning(pyplot):

Copy link

Member

lucyleeowJul 17, 2025

Choose a reason for hiding this comment

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

I think these can be one test, that checks deprecation ofy_pred

sklearn/metrics/_plot/tests/test_det_curve_display.py Outdated

		display_y_pred = DetCurveDisplay.from_predictions(y_true, y_pred=y_score)

		assert_allclose(
		display_y_pred.fpr, [5.000000e-01, 5.000000e-01, 5.000000e-01, 2.220446e-16]

Copy link

Member

lucyleeowJul 17, 2025

Choose a reason for hiding this comment

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

I think we only need to test one offpr orfnr to ensure that we are passingy_pred through correctly.
Maybe instead of hard coding the numbers we can just calculate them usingdet_curve?

Copy link

Author

luiser1401Jul 17, 2025

Choose a reason for hiding this comment

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

I've deleted hardcoded values for these kind of test in al three classes.

luiser1401 added5 commits

July 17, 2025 13:57

refactor deprecation warning

ab89f6c

change versioning in todo

48383ca

reformat changelog

5655d1f

merge tests

a4ae098

delete hardcoded values from tests

d98568f

luiser1401 requested a review fromlucyleeow

July 17, 2025 14:18

jeremiedbb reviewed

Jul 18, 2025

View reviewed changes

Copy link

Member

jeremiedbb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR@luiser1401. Here are some comments

sklearn/metrics/_plot/det_curve.py

		@@ -1,10 +1,14 @@
		# Authors: The scikit-learn developers
		# SPDX-License-Identifier: BSD-3-Clause

Copy link

Member

jeremiedbbJul 18, 2025

Choose a reason for hiding this comment

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

please revert this unrelated change.

sklearn/metrics/_plot/precision_recall_curve.py

		@@ -1,10 +1,10 @@
		# Authors: The scikit-learn developers
		# SPDX-License-Identifier: BSD-3-Clause

Copy link

Member

jeremiedbbJul 18, 2025

Choose a reason for hiding this comment

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

Please revert this unrelated change.

sklearn/metrics/_plot/tests/test_det_curve_display.py


		assert disp.estimator_name == expected_clf_name
		assert disp.line_.get_label() == expected_clf_name


		# TODO(1.9): remove

Copy link

Member

jeremiedbbJul 18, 2025

Choose a reason for hiding this comment

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

Suggested change

	# TODO(1.9): remove
	# TODO(1.10): remove

sklearn/metrics/_plot/tests/test_precision_recall_display.py


		forsin ["top","right"]:
		assertdisplay.ax_.spines[s].get_visible()isnotdespine

		ifdespine:
		forsin ["bottom","left"]:
		assertdisplay.ax_.spines[s].get_bounds()== (0,1)


		# TODO(1.9): remove

Copy link

Member

jeremiedbbJul 18, 2025

Choose a reason for hiding this comment

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

Suggested change

	# TODO(1.9): remove
	# TODO(1.10): remove

sklearn/utils/_plotting.py

Comment on lines +422 to +440

		# TODO(1.9): remove after the end of the deprecation period of `y_pred`
		def _deprecate_y_pred_parameter(y_score, y_pred):
		"""Deprecate `y_pred` in favour of of `y_score`."""
		if y_score is not None and not (isinstance(y_pred, str) and y_pred == "deprecated"):
		raise ValueError(
		"`y_pred` and `y_score` cannot be both specified. Please use `y_score`"
		" only as `y_pred` is deprecated in 1.7 and will be removed in 1.10."
		)
		if not (isinstance(y_pred, str) and y_pred == "deprecated"):
		warnings.warn(
		(
		"y_pred is deprecated in 1.7 and will be removed in 1.10. "
		"Please use `y_score` instead."
		),
		FutureWarning,
		)
		return y_pred

		return y_score

Copy link

Member

jeremiedbbJul 18, 2025

Choose a reason for hiding this comment

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

ForRocCurveDisplay, the deprecation happened in 1.7 but the deprecations of this PR will be released with 1.8, so we need to add aversion argument to this function.

sklearn/utils/_plotting.py

		if y_score is not None and not (isinstance(y_pred, str) and y_pred == "deprecated"):
		raise ValueError(
		"`y_pred` and `y_score` cannot be both specified. Please use `y_score`"
		" only as `y_pred` is deprecated in 1.7 and will be removed in 1.10."

Copy link

Member

jeremiedbbJul 18, 2025

Choose a reason for hiding this comment

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

Suggested change

	" only as `y_pred`is deprecated in 1.7 and will be removed in 1.10."
	" only as `y_pred`was deprecated in 1.7 and will be removed in 1.10."

sklearn/utils/_plotting.py

		if not (isinstance(y_pred, str) and y_pred == "deprecated"):
		warnings.warn(
		(
		"y_pred is deprecated in 1.7 and will be removed in 1.10. "

Copy link

Member

jeremiedbbJul 18, 2025

Choose a reason for hiding this comment

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

Suggested change

	"y_predis deprecated in 1.7 and will be removed in 1.10. "
	"y_predwas deprecated in 1.7 and will be removed in 1.10. "

doc/whats_new/upcoming_changes/sklearn.metrics/31764.fix.rst

Comment on lines +3 to +4

		:func:`metrics.PrecisionRecallDisplay.from_predictions`. Both will be removed in
		v1.10.

Copy link

Member

jeremiedbbJul 18, 2025

Choose a reason for hiding this comment

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

Suggested change

	:func:`metrics.PrecisionRecallDisplay.from_predictions`.Both will be removed in
	v1.10.
	:func:`metrics.PrecisionRecallDisplay.from_predictions`.`y_pred` will be removed in
	v1.10.

Labels

Documentation module:metrics

3 participants

Movatterモバイル変換

Uh oh!

FIX: Replace y_pred with y_score in DetCurveDisplay (issue: 31761)#31764

Are you sure you want to change the base?

FIX: Replace y_pred with y_score in DetCurveDisplay (issue: 31761)#31764

Conversation

luiser1401 commentedJul 15, 2025

Uh oh!

github-actionsbot commentedJul 15, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

lucyleeow left a comment• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremiedbb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actionsbot commentedJul 15, 2025•
edited
Loading

lucyleeow left a comment•
edited
Loading