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] load_breast_cancer dataset: added return_X_y option #7152

Merged
merged 5 commits into from Aug 6, 2016

Conversation

Projects
None yet
3 participants
@manu-chroma
Copy link
Contributor

manu-chroma commented Aug 5, 2016

Reference Issue

Partially Fixes #6670

What does this implement/fix? Explain your changes.

  1. added return_X_y option for load_breast_cancer dataset
  2. Wrote tests for the same

Any other comments?

Very similar to #7049

@manu-chroma manu-chroma changed the title [MRG + 1] load_breast_cancer dataset: added return_X_y option [MRG] load_breast_cancer dataset: added return_X_y option Aug 5, 2016

@@ -332,6 +332,14 @@ def load_breast_cancer():
Features real, positive
================= ==============
Parameters
----------
return_X_y : boolean, deafult=True

This comment has been minimized.

Copy link
@nelson-liu

nelson-liu Aug 5, 2016

Contributor

deafult -> default

This comment has been minimized.

Copy link
@nelson-liu

nelson-liu Aug 5, 2016

Contributor

also I believe default should be False?

@nelson-liu

This comment has been minimized.

Copy link
Contributor

nelson-liu commented Aug 5, 2016

You should also edit your previous entry in whats_new.rst to reflect the changes you've made here.

@manu-chroma

This comment has been minimized.

Copy link
Contributor Author

manu-chroma commented Aug 5, 2016

Fixed.

@@ -231,7 +231,7 @@ Enhancements
By `Sebastian Säger`_ and `YenChen Lin`_.

- Added new return type ``(data, target)`` : tuple option to
:func:`load_iris` dataset.
:func:`load_iris` dataset, :func:`load_breast_cancer` dataset
(`#7049 <https://github.com/scikit-learn/scikit-learn/pull/7049>`_) by

This comment has been minimized.

Copy link
@nelson-liu

nelson-liu Aug 5, 2016

Contributor

im inclined to have it formatted as such:
Add new return option to load (link to pr), loader2 (link to another pr), etc.

This comment has been minimized.

Copy link
@manu-chroma

manu-chroma Aug 5, 2016

Author Contributor

Yeah, it's a better formatting. Fixed.

@nelson-liu

This comment has been minimized.

Copy link
Contributor

nelson-liu commented Aug 5, 2016

LGTM, thanks!

@agramfort agramfort merged commit c98adf7 into scikit-learn:master Aug 6, 2016

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@agramfort

This comment has been minimized.

Copy link
Member

agramfort commented Aug 6, 2016

TomDLT added a commit to TomDLT/scikit-learn that referenced this pull request Oct 3, 2016

[MRG] load_breast_cancer dataset: added return_X_y option (scikit-lea…
…rn#7152)

* added return_X_y option to breast_cancer_dataset, tests included

* fix typo

* update whats_new

* removed extra space

* improved whats_new changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.