8000 numpy dev broken again? · Issue #7994 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

numpy dev broken again? #7994

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.

Sign up for GitHub

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

Jump to bottom
Closed
amueller opened this issue Dec 6, 2016 · 5 comments
Closed

numpy dev broken again? #7994

amueller opened this issue Dec 6, 2016 · 5 comments
Labels
Bug Easy Well-defined and straightforward way to resolve

Comments

@amueller
Copy link
Member
amueller commented Dec 6, 2016

Not sure if that was raised before but it seems the numpy dev travis is broken again because of string comparisons:

https://travis-ci.org/scikit-learn/scikit-learn/jobs/181514958
@amueller amueller added Bug Easy Well-defined and straightforward way to resolve Need Contributor labels Dec 6, 2016
@aashil
Copy link
Contributor
aashil commented Dec 6, 2016

I am taking this one.

@aashil
Copy link
Contributor
aashil commented Dec 6, 2016

I got the following warning while running the tests on local FutureWarning: elementwise comparison failed; returning scalar instead, but in the future will perform elementwise comparison which is because of line 93 if multioutput not in multioutput_options:

Do you think we should change the way we compare ? I was thinking of looping over the multioutput and check if each element is in multioutput_options.

@ivannz ivannz mentioned this issue Dec 7, 2016
Open
@lesteve
Copy link
Member
lesteve commented Dec 7, 2016 •
edited
Loading

FutureWarning: elementwise comparison failed; returning scalar instead, but in the future will perform elementwise comparison which is because of line 93 if multioutput not in multioutput_options:

This is the line causing the error with numpy master indeed. I think I would replace it with something like this (untested):

if isinstance(multioutput, six.string_types) and multioutput not in multioutput_options:
    # raise meaningful error message
elif multitoutput is not None:
    multioutput = check_array(multioutput, ensure_2d=False)
    # rest of the code goes here

The thing is that array comparison is a bit finicky in master, so it is easier to do simpler cases before more complicated ones (is it a string, if not a string is it None, otherwise generic case with check_array)

@lesteve
Copy link
Member
lesteve commented Dec 7, 2016 •
edited
Loading

@aashil please open a PR and we can discuss the code changes there in more details. I would advise you to install numpy from master to work on this issue provided you are not too uncomfortable with the idea.

@aashil aashil mentioned this issue Dec 8, 2016
Merged
@amueller
Copy link
Member Author

fixed in #8011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Easy Well-defined and straightforward way to resolve
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@amueller @lesteve @aashil
0