Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG+2] FIX: force consistency outputs of DummyClassifier's predict_proba when strategy is stratified #13266

Merged

Conversation

@chkoar
Copy link
Contributor

@chkoar chkoar commented Feb 25, 2019

What does this implement/fix? Explain your changes.

The DummyClassifier's predict_proba method returns a float64 array in all strategies except the stratified strategy which returns a int32 array.

Copy link
Contributor

@glemaitre glemaitre left a comment

LGTM. Could you add an entry to the what's new since it is impacting the end-user.

model.fit(X, y)
probas = model.predict_proba(X)

assert probas.dtype == np.float

This comment has been minimized.

@glemaitre

glemaitre Feb 26, 2019
Contributor

Suggested change
assert probas.dtype == np.float
assert probas.dtype == np.float64
y = [0, 2, 1, 1]
X = [[0]] * 4
model = DummyClassifier(strategy=strategy, random_state=0, constant=0)
model.fit(X, y)

This comment has been minimized.

@glemaitre

glemaitre Feb 26, 2019
Contributor

Suggested change
model.fit(X, y)
probas = model.fit(X, y).predict_proba(X)
])
def test_dtype_of_classifier_probas(strategy):
y = [0, 2, 1, 1]
X = [[0]] * 4

This comment has been minimized.

@glemaitre

glemaitre Feb 26, 2019
Contributor

Suggested change
X = [[0]] * 4
X = np.zeros(4)
@@ -709,3 +709,20 @@ def test_regressor_prediction_independent_of_X(strategy):
predictions2 = reg2.predict(X2)

assert_array_equal(predictions1, predictions2)


@pytest.mark.parametrize("strategy", [

This comment has been minimized.

@glemaitre

glemaitre Feb 26, 2019
Contributor

Suggested change
@pytest.mark.parametrize("strategy", [
@pytest.mark.parametrize(
"strategy", ["stratified", "most_frequent", "prior", "uniform", "constant"]
)

If it fits on the 80 characters it would be more compact

@@ -95,6 +95,14 @@ Support for Python 3.4 and below has been officially dropped.
``n_components`` is changed to ``min(n_features, n_classes - 1)`` if so.
Previously the change was made, but silently. :issue:`11526` by
:user:`William de Vazelhes<wdevazelhes>`.

:mod:`sklearn.dummy`
....................................

This comment has been minimized.

@glemaitre

glemaitre Feb 26, 2019
Contributor

Suggested change
....................................
....................
@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Feb 26, 2019

LGTM apart of long line in whats new

Copy link
Member

@amueller amueller left a comment

lgtm

@agramfort agramfort changed the title Fix dtype of DummyClassifier's predict_proba when strategy is stratified [MRG+2] Fix dtype of DummyClassifier's predict_proba when strategy is stratified Feb 26, 2019
@agramfort agramfort added this to Needs review in Sprint Paris 2019 Feb 26, 2019
@glemaitre glemaitre changed the title [MRG+2] Fix dtype of DummyClassifier's predict_proba when strategy is stratified [MRG+2] FIX: force consistency outputs of DummyClassifier's predict_proba when strategy is stratified Feb 26, 2019
@glemaitre glemaitre merged commit eb011b8 into scikit-learn:master Feb 26, 2019
11 of 16 checks passed
11 of 16 checks passed
LGTM analysis: C/C++ This pull request can't be analyzed because the commit could not be fetched from the repository
Details
LGTM analysis: JavaScript This pull request can't be analyzed because the commit could not be fetched from the repository
Details
LGTM analysis: Python This pull request can't be analyzed because the commit could not be fetched from the repository
Details
ci/circleci: deploy Your tests are queued behind your running builds
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
ci/circleci: doc Your tests passed on CircleCI!
Details
ci/circleci: doc-min-dependencies Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@azure-pipelines
scikit-learn.scikit-learn Build #20190226.73 succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux py35_conda_openblas) Linux py35_conda_openblas succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux py35_np_atlas) Linux py35_np_atlas succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux pylatest_conda) Linux pylatest_conda succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Windows py35) Windows py35 succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Windows py37) Windows py37 succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (macOS pylatest_conda) macOS pylatest_conda succeeded
Details
Sprint Paris 2019 automation moved this from Needs review to Done Feb 26, 2019
@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Feb 26, 2019

xhlulu added a commit to xhlulu/scikit-learn that referenced this pull request Apr 28, 2019
xhlulu added a commit to xhlulu/scikit-learn that referenced this pull request Apr 28, 2019
xhlulu added a commit to xhlulu/scikit-learn that referenced this pull request Apr 28, 2019
koenvandevelde added a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants