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] Fix tests on numpy master #8355

Merged
merged 1 commit into from Feb 15, 2017

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Feb 14, 2017

numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array. Very likely related to numpy/numpy#8441.

The first build in master I could find with the failure on numpy dev (from 2 days ago):
https://travis-ci.org/scikit-learn/scikit-learn/builds/200889357

import numpy as np
result = np.apply_along_axis(lambda X: np.array([[X[0]]]), 1, np.arange(6).reshape(3, -1))
print(result.shape)

outputs (3, 1, 1) on numpy master and (3, 1) on numpy 1.12.

A scikit-learn snippet closely related to the test failure:

from sklearn.gaussian_process import kernels
kernel = kernels.PairwiseKernel()
print(kernel.diag(np.arange(6).reshape(3, -1)).shape)

outputs (3, 1) on numpy master and (3,) on numpy 1.12.

@codecov
Copy link

codecov bot commented Feb 14, 2017

Codecov Report

Merging #8355 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    #8355   +/-   ##
=======================================
  Coverage   94.75%   94.75%           
=======================================
  Files         342      342           
  Lines       60801    60801           
=======================================
  Hits        57609    57609           
  Misses       3192     3192
Impacted Files Coverage Δ
sklearn/gaussian_process/kernels.py 91.54% <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 05b5b37...3578d4b. Read the comment docs.

@lesteve
Copy link
Member Author

lesteve commented Feb 14, 2017

The Travis build on this PR with numpy master is green as you can see from:
https://travis-ci.org/scikit-learn/scikit-learn/builds/201521405

@@ -1852,7 +1852,7 @@ def diag(self, X):
Diagonal of kernel k(X, X)
"""
# We have to fall back to slow way of computing diagonal
return np.apply_along_axis(self, 1, X)[:, 0]
return np.apply_along_axis(lambda arr: self(arr).ravel(), 1, X)[:, 0]
Copy link
Member

Choose a reason for hiding this comment

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

I think this is written in a confusing way, if self(arr) is returning an array of shape (1,1).

Shouldn't either of the following do the same?

np.apply_along_axis(lambda arr: self(arr)[0, 0], 1, X)
np.apply_along_axis(self, 1, X).ravel()

Copy link
Member Author

@lesteve lesteve Feb 15, 2017

Choose a reason for hiding this comment

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

I think this is written in a confusing way, if self(arr) is returning an array of shape (1,1).

Yeah I was not sure what was the best way to write it. Would you rather have

np.apply_along_axis(lambda arr: self(arr)[0, 0], 1, X)

Shouldn't either of the following do the same?

It does in numpy 1.12 but not in numpy master as I tried to highlight in the description.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually my bad you need to use:the following in numpy master:

np.apply_along_axis(lambda arr: self(arr)[0], 1, X)

@jnothman
Copy link
Member

jnothman commented Feb 15, 2017 via email

@lesteve
Copy link
Member Author

lesteve commented Feb 15, 2017

Ravelling the output doesn't wok without the lambda?

Ah sorry I read your comment too quickly above, this does work in numpy master indeed:

np.apply_along_axis(self, 1, X).ravel()

numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array
@jnothman
Copy link
Member

LGTM

@jnothman jnothman merged commit b89fcd3 into scikit-learn:master Feb 15, 2017
@lesteve lesteve deleted the fix-tests-on-numpy-master branch February 15, 2017 12:22
@lesteve
Copy link
Member Author

lesteve commented Feb 15, 2017

Thanks!

sergeyf pushed a commit to sergeyf/scikit-learn that referenced this pull request Feb 28, 2017
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array
@Przemo10 Przemo10 mentioned this pull request Mar 17, 2017
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array
jakirkham pushed a commit to jakirkham/scikit-learn that referenced this pull request Jun 15, 2017
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array
amueller pushed a commit that referenced this pull request Jun 19, 2017
* Fix tests on numpy master (#7946)

Until now we were in a edge case on assert_array_equal

* Fix tests on numpy master (#8355)

numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array

* [MRG] Updated plot_stock_market.py to use Google Finance (#9010)

* DOC updated plot_stock_market.py to use Google Finance

The implementations is intentionally very basic not to distract the users
from the example. Specifically unlike ``quotes_historical_yahoo_ochl`` it
does not cache downloaded data.

I also had to remove some symbols because the have no data on Google for
the specified date interval. These are WBA, LMT, KFT and MTU.

Closes #8899

* DOC removed plot_stock_market.py from expected failing examples

* Addressed review comments

* Addressed another pass of review comments

* [MRG] Remove DeprecationWarnings in examples due to using floats instead of ints (#8040)
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array
lemonlaug pushed a commit to lemonlaug/scikit-learn that referenced this pull request Jan 6, 2021
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array
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.

None yet

2 participants