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+1] fix P/R/F for truncated range(n_labels) #10377

Merged
merged 4 commits into from Jan 11, 2018

Conversation

Projects
None yet
3 participants
@gxyd
Contributor

gxyd commented Dec 27, 2017

What does this implement/fix? Explain your changes.

Fixes #10307

for ex. if n_labels = 5, then passing labels = [0, 1, 2]
will give results similar to labels = [0, 1, 2, 3, 4], neglecting
the value of labels.

Currently in master branch:

>>> y_true = np.array([[0, 1, 1], [1, 0, 0]])
>>> y_pred = np.array([[1, 1, 1], [1, 0, 1]])
>>> precision_recall_fscore_support(y_true, y_pred, average='samples', labels=[0, 1])
(0.58333333333333326, 1.0, 0.73333333333333339, None)
>>> precision_recall_fscore_support(y_true, y_pred, average='samples', labels=[1, 0])
(0.75, 1.0, 0.83333333333333326, None)

I'm not sure if this will require any changes to test_common, since labels=[0, 1] and labels=[1, 0] should give the same result for average='samples' (atleast for average='samples', haven't thought about other averages), this is similar to commutative property w.r.t. labels.

fix P/R/F for truncated range(n_labels)
for ex. if n_labels = 5, then passing labels = [0, 1, 2]
will give results similar to labels = [0, 1, 2, 3, 4], neglecting
the value of labels

@gxyd gxyd changed the title from fix P/R/F for truncated range(n_labels) to [WIP] fix P/R/F for truncated range(n_labels) Dec 27, 2017

@gxyd gxyd changed the title from [WIP] fix P/R/F for truncated range(n_labels) to [MRG] fix P/R/F for truncated range(n_labels) Dec 27, 2017

@jnothman

otherwise LGTM

Show outdated Hide outdated sklearn/metrics/tests/test_classification.py Outdated
@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Dec 31, 2017

Member

Please add an entry to the change log at doc/whats_new/v0.20.rst. Like the other entries there, please reference this pull request with :issue: and credit yourself (and other contributors if applicable) with :user:

Member

jnothman commented Dec 31, 2017

Please add an entry to the change log at doc/whats_new/v0.20.rst. Like the other entries there, please reference this pull request with :issue: and credit yourself (and other contributors if applicable) with :user:

@jnothman jnothman changed the title from [MRG] fix P/R/F for truncated range(n_labels) to [MRG+1] fix P/R/F for truncated range(n_labels) Dec 31, 2017

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Dec 31, 2017

Contributor

I've made the asked change, though I've commented #PR number instead of #issue_number. I think that is fine as well.

Contributor

gxyd commented Dec 31, 2017

I've made the asked change, though I've commented #PR number instead of #issue_number. I think that is fine as well.

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Dec 31, 2017

Contributor

Fixes #10307

Contributor

gxyd commented Dec 31, 2017

Fixes #10307

gxyd added some commits Dec 31, 2017

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Dec 31, 2017

Member

Flake8 errors?

Member

jnothman commented Dec 31, 2017

Flake8 errors?

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Jan 1, 2018

Contributor

Doesn't it seem like so to me. It says:

 l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/build/scikit-learn/scikit-learn/sklearn/utils/deprecation.py:58: DeprecationWarning: Class VBGMM is deprecated; The `VBGMM` class is not working correctly and it's better to use `sklearn.mixture.BayesianGaussianMixture` class with parameter `weight_concentration_prior_type='dirichlet_distribution'` instead. VBGMM is deprecated in 0.18 and will be removed in 0.20.
  warnings.warn(msg, category=DeprecationWarning)
collecting 7727 items/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
collecting 7999 items/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
collected 8423 items 

....repeated lines

The job exceeded the maximum time limit for jobs, and has been terminated.

Here is the link to failing travis job https://travis-ci.org/scikit-learn/scikit-learn/jobs/323659077 . Some random failure?

Contributor

gxyd commented Jan 1, 2018

Doesn't it seem like so to me. It says:

 l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
/home/travis/build/scikit-learn/scikit-learn/sklearn/utils/deprecation.py:58: DeprecationWarning: Class VBGMM is deprecated; The `VBGMM` class is not working correctly and it's better to use `sklearn.mixture.BayesianGaussianMixture` class with parameter `weight_concentration_prior_type='dirichlet_distribution'` instead. VBGMM is deprecated in 0.18 and will be removed in 0.20.
  warnings.warn(msg, category=DeprecationWarning)
collecting 7727 items/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
collecting 7999 items/home/travis/miniconda/envs/testenv/lib/python3.4/site-packages/_pytest/python.py:625: PendingDeprecationWarning: This usage is deprecated, please use pytest.Function instead
  l.append(self.Function(name, self, args=args, callobj=call))
collected 8423 items 

....repeated lines

The job exceeded the maximum time limit for jobs, and has been terminated.

Here is the link to failing travis job https://travis-ci.org/scikit-learn/scikit-learn/jobs/323659077 . Some random failure?

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Jan 1, 2018

Member
Member

jnothman commented Jan 1, 2018

@glemaitre glemaitre merged commit 60b0cf8 into scikit-learn:master Jan 11, 2018

0 of 5 checks passed

ci/circleci: python2 Your tests are queued behind your running builds
Details
ci/circleci: python3 Your tests are queued behind your running builds
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
lgtm analysis: Python Running analyses for revisions
Details
@glemaitre

This comment has been minimized.

Show comment
Hide comment
@glemaitre

glemaitre Jan 11, 2018

Contributor

@gxyd Thanks!!!

Contributor

glemaitre commented Jan 11, 2018

@gxyd Thanks!!!

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Jan 11, 2018

Contributor

@glemaitre was it necessary the changes in commit that you added?

Contributor

gxyd commented Jan 11, 2018

@glemaitre was it necessary the changes in commit that you added?

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Jan 11, 2018

Contributor

@glemaitre never mind. I just saw your comment here #10377 (comment) .

Thanks @jnothman @glemaitre for review.

Contributor

gxyd commented Jan 11, 2018

@glemaitre never mind. I just saw your comment here #10377 (comment) .

Thanks @jnothman @glemaitre for review.

@glemaitre

This comment has been minimized.

Show comment
Hide comment
@glemaitre

glemaitre Jan 11, 2018

Contributor

@gxyd I decided that it was not worth to make a round of change/review for such a nitpick :)
Regarding the change itself, I would think that we should try writing python 3 code which is back-compatible but this is just a detail.

Contributor

glemaitre commented Jan 11, 2018

@gxyd I decided that it was not worth to make a round of change/review for such a nitpick :)
Regarding the change itself, I would think that we should try writing python 3 code which is back-compatible but this is just a detail.

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Jan 11, 2018

Member
Member

jnothman commented Jan 11, 2018

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