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] ENH Adding various averages to classification_report #11679

Merged
merged 17 commits into from Aug 7, 2018

Conversation

Projects
None yet
4 participants
@qinhanmin2014
Member

qinhanmin2014 commented Jul 25, 2018

Closes #4558
Closes #4622
Closes #10029
Closes #11657

(1) Support dictionary output
(2) Reorganize to avoid duplicate code
(3) Return int/float instead of np.int/np.float in classification_report (See #11657)

apacha and others added some commits Oct 27, 2017

Adding average-option to classification_report to make default more e…
…xplicit and allow changing it easily for the report (#4558).
Replacing remaining occurencies of total/avg in documentation with "w…
…eighted avg" and improving style to comply with Style-guidelines (line-width <80 chars).
Merge remote-tracking branch 'upstream/master'
# Conflicts:
#	sklearn/metrics/classification.py
Reporting all averages (micro, macro, weighted and sample (if applica…
…ble)) when calling classification_report.

Updated numbers in working_with_text_data tutorial, because the data seems to have changed.
Merge branch 'master' into pr/10029
# Conflicts:
#	sklearn/metrics/classification.py

@qinhanmin2014 qinhanmin2014 added this to the 0.20 milestone Jul 25, 2018

@qinhanmin2014 qinhanmin2014 changed the title from ENH Adding various averages to classification_report to [MRG] ENH Adding various averages to classification_report Jul 25, 2018

@qinhanmin2014

This comment has been minimized.

Show comment
Hide comment
@qinhanmin2014

qinhanmin2014 Jul 25, 2018

Member

cc reviewers from the original PR
@jnothman @glemaitre @GaelVaroquaux @amueller
Note that this is not backward compatible, i.e., users cannot get the old output. But I think that's acceptable, since users are actually getting more information here.

Member

qinhanmin2014 commented Jul 25, 2018

cc reviewers from the original PR
@jnothman @glemaitre @GaelVaroquaux @amueller
Note that this is not backward compatible, i.e., users cannot get the old output. But I think that's acceptable, since users are actually getting more information here.

@jnothman

Was there something particular I was meant to see / look at?

@qinhanmin2014

This comment has been minimized.

Show comment
Hide comment
@qinhanmin2014

qinhanmin2014 Jul 26, 2018

Member

Thanks @jnothman for the review :)

Was there something particular I was meant to see / look at?

For you, maybe no, since you've approved #10029 and #11657. The main purpose of this PR is to support average option for dictionary output.

Member

qinhanmin2014 commented Jul 26, 2018

Thanks @jnothman for the review :)

Was there something particular I was meant to see / look at?

For you, maybe no, since you've approved #10029 and #11657. The main purpose of this PR is to support average option for dictionary output.

@qinhanmin2014

This comment has been minimized.

Show comment
Hide comment
@qinhanmin2014

qinhanmin2014 Aug 7, 2018

Member

I'll count my +1 and merge when green. Hope that users won't complain about the backward incompatible.

Member

qinhanmin2014 commented Aug 7, 2018

I'll count my +1 and merge when green. Hope that users won't complain about the backward incompatible.

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Aug 7, 2018

Member
Member

jnothman commented Aug 7, 2018

@qinhanmin2014 qinhanmin2014 merged commit 0efaf2d into scikit-learn:master Aug 7, 2018

7 checks passed

ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: python2 Your tests passed on CircleCI!
Details
ci/circleci: python3 Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 93.08%)
Details
codecov/project 93.08% (+<.01%) compared to aee6759
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@qinhanmin2014

This comment has been minimized.

Show comment
Hide comment
@qinhanmin2014
Member

qinhanmin2014 commented Aug 7, 2018

Thanks @apacha

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