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

Projects
None yet
2 participants
@lesteve
Member

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

This comment has been minimized.

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

This comment has been minimized.

Member

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]

This comment has been minimized.

@jnothman

jnothman Feb 15, 2017

Member

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()

This comment has been minimized.

@lesteve

lesteve Feb 15, 2017

Member

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.

This comment has been minimized.

@lesteve

lesteve Feb 15, 2017

Member

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

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

This comment has been minimized.

Member

jnothman commented Feb 15, 2017

@lesteve

This comment has been minimized.

Member

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()
Fix tests on numpy master
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array
@jnothman

This comment has been minimized.

Member

jnothman commented Feb 15, 2017

LGTM

@jnothman jnothman merged commit b89fcd3 into scikit-learn:master Feb 15, 2017

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 94.75%)
Details
codecov/project 94.75% (+0%) compared to 05b5b37
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lesteve lesteve deleted the lesteve:fix-tests-on-numpy-master branch Feb 15, 2017

@lesteve

This comment has been minimized.

Member

lesteve commented Feb 15, 2017

Thanks!

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

Fix tests on numpy master (scikit-learn#8355)
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array

@Przemo10 Przemo10 referenced this pull request Mar 17, 2017

Closed

update fork (#1) #8606

@jakirkham jakirkham referenced this pull request Jun 12, 2017

Closed

Add NumPy 1.13 #47

Sundrique added a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017

Fix tests on numpy master (scikit-learn#8355)
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array

jakirkham added a commit to jakirkham/scikit-learn that referenced this pull request Jun 15, 2017

Fix tests on numpy master (scikit-learn#8355)
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array

amueller added a commit that referenced this pull request Jun 19, 2017

Backport NumPy 1.13.0 fixes to 0.18.X (#9137)
* 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 added a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017

Fix tests on numpy master (scikit-learn#8355)
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array

paulha added a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017

Fix tests on numpy master (scikit-learn#8355)
numpy.apply_along_axis has changed behaviour when the function passed
in returns a 2d array

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

Fix tests on numpy master (scikit-learn#8355)
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