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] correct comparison in GaussianNB for 'priors' #10005

Merged
merged 2 commits into from Oct 26, 2017

Conversation

Projects
None yet
3 participants
@gxyd
Contributor

gxyd commented Oct 25, 2017

Reference Issues/PRs

Fixes #9633

What does this implement/fix? Explain your changes.

correct comparison in GaussianNB for 'priors'

Any other comments?

I am not fully sure if I have written the test as correctly since, I have written it as clf.fit(X, Y) and I don't know if I should use assert_.... or not. That is why I have not put MRG on title of PR.

@jnothman

I've not yet double-checked that this test fails on master (does it?), but this otherwise lgtm

Show outdated Hide outdated sklearn/tests/test_naive_bayes.py
Show outdated Hide outdated sklearn/tests/test_naive_bayes.py

@jnothman jnothman changed the title from correct comparison in GaussianNB for 'priors' to [MRG+1] correct comparison in GaussianNB for 'priors' Oct 25, 2017

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Oct 26, 2017

Contributor

I have addressed the comments and fixed the failing flake8 errors. Do I need to squash the commits? (I have 2 commits now) Was wondering if you follow some strict policy against 'squashing' public commits.

Contributor

gxyd commented Oct 26, 2017

I have addressed the comments and fixed the failing flake8 errors. Do I need to squash the commits? (I have 2 commits now) Was wondering if you follow some strict policy against 'squashing' public commits.

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Oct 26, 2017

Member
Member

jnothman commented Oct 26, 2017

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Oct 26, 2017

Contributor
Contributor

gxyd commented Oct 26, 2017

@TomDLT

This comment has been minimized.

Show comment
Hide comment
@TomDLT

TomDLT Oct 26, 2017

Member

I've not yet double-checked that this test fails on master

Test does fail on master.

Squashed and merged, thanks @gxyd !

Member

TomDLT commented Oct 26, 2017

I've not yet double-checked that this test fails on master

Test does fail on master.

Squashed and merged, thanks @gxyd !

@TomDLT TomDLT merged commit e41c4d5 into scikit-learn:master Oct 26, 2017

6 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 96.17%)
Details
codecov/project 96.18% (+<.01%) compared to 102620f
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details

@gxyd gxyd deleted the gxyd:check-priors branch Oct 26, 2017

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Oct 26, 2017

Contributor

Thanks @TomDLT . This was my first contribution to scikit-learn.

Contributor

gxyd commented Oct 26, 2017

Thanks @TomDLT . This was my first contribution to scikit-learn.

@TomDLT

This comment has been minimized.

Show comment
Hide comment
@TomDLT

TomDLT Oct 26, 2017

Member

Congrats on your first contribution, hoping to see you around again !

Member

TomDLT commented Oct 26, 2017

Congrats on your first contribution, hoping to see you around again !

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Oct 26, 2017

Contributor

I'll be here.

Contributor

gxyd commented Oct 26, 2017

I'll be here.

srajanpaliwal added a commit to srajanpaliwal/scikit-learn that referenced this pull request Oct 26, 2017

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Oct 26, 2017

Member

Forgot a changelog entry. @gxyd do you mind submitting a new PR with a change to doc/whats_new?

Member

jnothman commented Oct 26, 2017

Forgot a changelog entry. @gxyd do you mind submitting a new PR with a change to doc/whats_new?

@gxyd

This comment has been minimized.

Show comment
Hide comment
@gxyd

gxyd Oct 27, 2017

Contributor
Contributor

gxyd commented Oct 27, 2017

amueller added a commit that referenced this pull request Oct 27, 2017

[MRG+1] add changelog entry for fixed and merged PR #10005 issue #9633 (
#10025)

* add changelog entry for fixed and merged PR #10005 issue #9633

* change name

* change PR number

donigian added a commit to donigian/scikit-learn that referenced this pull request Oct 28, 2017

Merge branch 'master' of github.com:scikit-learn/scikit-learn into do…
…cs/donigian-update-contribution-guidelines

* 'master' of github.com:scikit-learn/scikit-learn: (23 commits)
  fixes scikit-learn#10031: fix attribute name and shape in documentation (scikit-learn#10033)
  [MRG+1] add changelog entry for fixed and merged PR scikit-learn#10005 issue scikit-learn#9633 (scikit-learn#10025)
  [MRG] Fix LogisticRegression see also should include LogisticRegressionCV(scikit-learn#9995) (scikit-learn#10022)
  [MRG + 1] Labels of clustering should start at 0 or -1 if noise (scikit-learn#10015)
  MAINT Remove redundancy in scikit-learn#9552 (scikit-learn#9573)
  [MRG+1] correct comparison in GaussianNB for 'priors' (scikit-learn#10005)
  [MRG + 1] ENH add check_inverse in FunctionTransformer (scikit-learn#9399)
  [MRG] FIX bug in nested set_params usage (scikit-learn#9999)
  [MRG+1] Fix LOF and Isolation benchmarks (scikit-learn#9798)
  [MRG + 1] Fix negative inputs checking in mean_squared_log_error (scikit-learn#9968)
  DOC Fix typo (scikit-learn#9996)
  DOC Fix typo: x axis -> y axis (scikit-learn#9985)
  improve example plot_forest_iris.py (scikit-learn#9989)
  [MRG+1] Deprecate pooling_func unused parameter in AgglomerativeClustering (scikit-learn#9875)
  DOC update news
  DOC Fix three typos in manifold documentation (scikit-learn#9990)
  DOC add missing dot in docstring
  DOC Add what's new for 0.19.1 (scikit-learn#9983)
  Improve readability of outlier detection example. (scikit-learn#9973)
  DOC: Fixed typo (scikit-learn#9977)
  ...

maskani-moh added a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017

maskani-moh added a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017

[MRG+1] add changelog entry for fixed and merged PR scikit-learn#10005
…issue scikit-learn#9633 (scikit-learn#10025)

* add changelog entry for fixed and merged PR scikit-learn#10005 issue scikit-learn#9633

* change name

* change PR number

jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017

jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017

[MRG+1] add changelog entry for fixed and merged PR scikit-learn#10005
…issue scikit-learn#9633 (scikit-learn#10025)

* add changelog entry for fixed and merged PR scikit-learn#10005 issue scikit-learn#9633

* change name

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