Add NumPy 1.13 #47
Add NumPy 1.13 #47
Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
Re-render to add NumPy 1.13 support.
|
Seeing some build failures on CircleCI with Python 2.7 and NumPy 1.13. Though presumably will see the same issues with other Python versions as well. Any ideas as to what is going on? ======================================================================
FAIL: sklearn.gaussian_process.tests.test_kernels.test_kernel_diag
----------------------------------------------------------------------
Traceback (most recent call last):
File "/feedstock_root/build_artefacts/scikit-learn_1497021226636/_t_env/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
self.test(*self.arg)
File "/feedstock_root/build_artefacts/scikit-learn_1497021226636/_t_env/lib/python2.7/site-packages/sklearn/gaussian_process/tests/test_kernels.py", line 142, in test_kernel_diag
assert_almost_equal(K_call_diag, K_diag, 5)
File "/feedstock_root/build_artefacts/scikit-learn_1497021226636/_t_env/lib/python2.7/site-packages/numpy/testing/utils.py", line 563, in assert_almost_equal
return assert_array_almost_equal(actual, desired, decimal, err_msg)
File "/feedstock_root/build_artefacts/scikit-learn_1497021226636/_t_env/lib/python2.7/site-packages/numpy/testing/utils.py", line 962, in assert_array_almost_equal
precision=decimal)
File "/feedstock_root/build_artefacts/scikit-learn_1497021226636/_t_env/lib/python2.7/site-packages/numpy/testing/utils.py", line 715, in assert_array_compare
raise AssertionError(msg)
AssertionError:
Arrays are not almost equal to 5 decimals
(shapes (5,), (5, 1) mismatch)
x: array([ 2.84495e+06, 1.71974e+07, 7.08257e+06, 6.79164e+04,
6.60218e+02])
y: array([[ 2.84495e+06],
[ 1.71974e+07],
[ 7.08257e+06],...
======================================================================
FAIL: sklearn.model_selection.tests.test_split.test_cv_iterable_wrapper
------------Edit: Have raised this to upstream in issue ( scikit-learn/scikit-learn#9094 ). |
|
thanks :) |
| @@ -13,6 +13,9 @@ source: | |||
| patches: | |||
| # TODO: Remove this patch when 0.19 is released | |||
| - skip-tests-failing-on-0.18.1.patch | |||
| # TODO: Remove this patch when 0.19 is released | |||
| - PR_7946.diff | |||
| - PR_8355.diff | |||
There was a problem hiding this comment.
|
So I had added PR ( scikit-learn/scikit-learn#8355 ), which tweaks the code for NumPy 1.13 (not a test) as it fixed one of the test failures. Is that ok? |
|
I'm happy with it, I think it's reasonable to port this here. I'm not sure if @GaelVaroquaux or @ogrisel or @jnothman have opinions. |
|
Also @lesteve? |
As I tried to argue a few times before, I am not in favour of changing the scikit-learn code outside of tests in conda-forge. I want the scikit-learn code to be identical when you install from pip, conda without conda-forge or conda with conda-forge. I would just skip the relevant test i.e. which is sklearn.gaussian_process.tests.test_kernels.test_kernel_diag. |
|
@lesteve in general I agree. The problem is that skipping the test exposes the user to a bug. I didn't want to do 0.18.2 for this given that we'll do 0.19 within a month. |
|
I think that this case is a bit different. Before we were trying to do a release and had a bunch of test failures. While there were patches that fixed the issues, they did also have some impact on the algorithms themselves. There was a fair question of whether that should happen in the conda-forge build. I think we came to a reasonable solution there and am quite ok with it. 😄 In this case, we are trying to build with a new version of NumPy and protect users from changed behavior caused by NumPy. The results themselves should be identical to what they were on previous NumPy versions or previous builds of the package. So it doesn't really fall in the category of an algorithmic change. However, if we don't do this patch to fix this behavior with NumPy 1.13, it unfortunately raises the question of whether conda-forge should be building this version with NumPy 1.13. Personally I'm leaning towards no. 😞 As to alternatives, maybe a patch release is in order? This would get rid of the skipped test patch and provide NumPy 1.13 support by virtue of including the fixes. Not entirely sure what goes into it on your end. However if this patch is considered problematic, perhaps a patch release is a reasonable solution. |
|
Since 0.19 should happen not that far in the future, I am fine waiting for scikit-learn 0.19 release before building with numpy 1.13 on conda-forge. |
|
A bugfix release wouldn't be that hard... 0.19 will likely be done within the next month. Though I can also just put together a bugfix release, it shouldn't be that hard. |
|
+1 for an upstream bugfix release rather than a conda-forge specific patch. |
|
@ogrisel You said something different over the weekend ;) well I guess not that explicitly. But maybe I can do it tomorrow? But we only fix this bug, not all of them? Or all that cherry-pick easily? (that would be way more work, I think) |
|
Have added PR ( scikit-learn/scikit-learn#9137 ) to backport NumPy 1.13.0 fixes in the patches above. Hopefully this helps with the patch release. |
|
Closing as this is being replaced with PR ( #50 ), which contains the 0.18.2. Thanks everyone for your hard work in getting this together. 😄 |
Re-render with
conda-smithy2.3.1 to add NumPy 1.13 to CI matrices.