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

Accept column vectors when having binary or multiclass targets #673

Merged
merged 10 commits into from Feb 2, 2020

Conversation

chkoar
Copy link
Member

@chkoar chkoar commented Jan 7, 2020

Reference Issue

Fixes #671

What does this implement/fix? Explain your changes.

Converts the target vector in the binary or multiclass case

Any other comments?

#666 (OMG what an evil number) is related but it will be solved probably in a following PR because there is an extra check of the name in the y which I think that is redundant.

@chkoar chkoar requested a review from glemaitre Jan 7, 2020
@chkoar chkoar changed the title Accept column vectors when having binary or multiclass targets [WIP] Accept column vectors when having binary or multiclass targets Jan 7, 2020
@codecov
Copy link

codecov bot commented Jan 31, 2020

Codecov Report

No coverage uploaded for pull request base (master@ebc25b4). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #673   +/-   ##
=========================================
  Coverage          ?   98.59%           
=========================================
  Files             ?       82           
  Lines             ?     4850           
  Branches          ?        0           
=========================================
  Hits              ?     4782           
  Misses            ?       68           
  Partials          ?        0
Impacted Files Coverage Δ
...election/tests/test_instance_hardness_threshold.py 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebc25b4...d210ac0. Read the comment docs.

@pep8speaks
Copy link

pep8speaks commented Jan 31, 2020

Hello @chkoar! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-01-31 16:24:16 UTC

random_state=0,
)

y = y.reshape(-1, 1) # Make the target 2d
Copy link
Member

@glemaitre glemaitre Jan 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add another check when having pandas Series and DataFrame

Copy link
Member Author

@chkoar chkoar Jan 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking to make a follow up PR to address #666. So, this check could be added in that same PR. What do you think?

Copy link
Member

@glemaitre glemaitre Feb 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK good then

@chkoar
Copy link
Member Author

chkoar commented Jan 31, 2020

@glemaitre after a retrigger all tests passed.

@chkoar chkoar requested a review from glemaitre Jan 31, 2020
@chkoar chkoar changed the title [WIP] Accept column vectors when having binary or multiclass targets Accept column vectors when having binary or multiclass targets Jan 31, 2020
@glemaitre glemaitre merged commit 42cd496 into scikit-learn-contrib:master Feb 2, 2020
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants