compatibility with scipy 0.19 #15689

zym1010 · 2017-03-14T18:52:27Z

jreback

minor comment
pls add a whatsnew entry (say compat with scipy 0.19.0)
those skips look fine.

jreback · 2017-03-14T18:59:41Z

pandas/core/window.py

@@ -544,7 +544,9 @@ def _pop_args(win_type, arg_names, kwargs):
                return all_args

            win_type = _validate_win_type(self.win_type, kwargs)
-            return sig.get_window(win_type, window).astype(float)
+            # GH #15662.


you don't need these comments

alright. so no comment at all?

yea you can take it out, unless passing the arg is meaningful?

what do you mean by "meaningful"?

if it were non-obvious what this parameter means / did (but passing it as False). So on 2nd thought if you can provide a 1-line comment on the meaning would be good.

jreback · 2017-03-14T19:03:09Z

note that there are more skips

pandas/tests//frame/test_missing.py:        tm.skip_if_no_package('scipy', max_version='0.19.0')
pandas/tests//series/test_missing.py:        tm.skip_if_no_package('scipy', max_version='0.19.0')
pandas/tests//test_window.py:        tm.skip_if_no_package('scipy', max_version='0.19.0')
pandas/tests//test_window.py:        tm.skip_if_no_package('scipy', max_version='0.19.0')
pandas/tests//test_window.py:        tm.skip_if_no_package('scipy', max_version='0.19.0')
pandas/tests//test_window.py:        tm.skip_if_no_package('scipy', max_version='0.19.0')
pandas/tests//test_window.py:        tm.skip_if_no_package('scipy', max_version='0.19.0')

This ones ok

pandas/tests//sparse/test_frame.py:    tm.skip_if_no_package('scipy', max_version='0.19.0')

if any remain (IOW can't / not easy fix), then we should switch these to xfailing (in 0.19.0), but that's a bit more tricky.

zym1010 · 2017-03-14T19:07:22Z

@jreback

note that there are more skips

I've fixed 5 of them, and plan to fix 2 more.

pandas/tests//sparse/test_frame.py never appears in my original issue. So not sure what to do with it.

jreback · 2017-03-14T19:09:20Z

@zym1010 sure np. Yeah ignore the sparse one; I added that because of an incompat with certain types of sparse matrices in 0.19.0 (so not relevant to this issue; it was another change in scipy 0.19.0).

zym1010 · 2017-03-14T19:38:12Z

@jreback function-wise, it's now fixed. I will fix the comment later.

jreback · 2017-03-14T20:24:01Z

pandas/tests/frame/test_missing.py

@@ -561,8 +561,13 @@ def test_interp_various(self):
        assert_frame_equal(result, expected)

        result = df.interpolate(method='cubic')
-        expected.A.loc[3] = 2.81621174
-        expected.A.loc[13] = 5.64146581
+        import scipy


you can define a variable at the very top of the file

try: import scipy _is_scipy_ge_0190 = scipy.__version__ >= LooseVersion(....) exept: _is_scipy_ge_0190 = False

jreback · 2017-03-14T20:24:49Z

pandas/tests/frame/test_missing.py

-        expected.A.loc[13] = 5.64146581
+        import scipy
+        if scipy.__version__ >= LooseVersion('0.19.0'):
+            expected.A.loc[3] = 2.81547781


also can you add a comment here (1-liner), on what qualitatively changed (adding the link is ok too).

imagine you are future reader and you have no idea why this was done....

jreback · 2017-03-14T21:57:35Z

looks like just a couple of linting issues

git diff master | flake8 --diff will show them locally.

add a whatsnew and looks good to go.

codecov · 2017-03-15T00:36:36Z

Codecov Report

Merging #15689 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #15689      +/-   ##
==========================================
- Coverage   91.02%      91%   -0.03%     
==========================================
  Files         143      143              
  Lines       49409    49409              
==========================================
- Hits        44977    44965      -12     
- Misses       4432     4444      +12

Impacted Files	Coverage Δ
pandas/core/window.py	`96.18% <100%> (ø)`	✅
pandas/io/gbq.py	`25% <0%> (-58.34%)`	❌
pandas/tools/merge.py	`93.68% <0%> (-0.34%)`	❌
pandas/util/testing.py	`80.66% <0%> (-0.19%)`	❌
pandas/core/frame.py	`97.87% <0%> (-0.1%)`	❌
pandas/core/common.py	`91.3% <0%> (+0.33%)`	✅

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 2cad4dd...3cc6528. Read the comment docs.

jreback · 2017-03-15T01:29:44Z

lgtm. ping on green.

jreback · 2017-03-15T13:30:59Z

thanks @zym1010

jreback · 2017-03-16T14:21:22Z

hmm, seems https://travis-ci.org/pandas-dev/pandas/jobs/211720204 is showing a failure on master scipy that does not show up on 0.19.0 :<

to test installing these look at ci/requirements-3.5_NUMPY_DEV.build.sh we are installing from wheels (so you have to create a separate environment to build this).

jreback · 2017-03-16T14:24:06Z

hmm, maybe they changed it back? (it looks like its compute the < 0.19.0) one? weird

zym1010 · 2017-03-16T14:41:03Z

@jreback no idea. I think for scipy 0.19, definitely it should not return the pre 0.19 result. I think it works with conda-forge version of Scipy. What's the difference between scipy 0.19 dev and that one?

zym1010 · 2017-03-16T14:47:57Z

I wonder if @ev-br has any idea on this.

ev-br · 2017-03-16T14:57:17Z

pandas/tests/frame/test_missing.py

+            expected.A.loc[13] = 6.12648668
+        else:
+            expected.A.loc[3] = 2.82533638
+            expected.A.loc[13] = 6.02817974


Generally, I'd recommend to avoid hard-coding the floating-point numbers like this: these are implementation details really. The values at round numbers should not change though. Or, at least, I'd use a relatively loose tolerance to smooth over these implementation details.

@ev-br point taken. Yet I think as this is the way pandas test suites work for a very long time, it should not break arbitrarily. These computation results should be pretty stable, across different architectures, etc., for a given scipy version.

@ev-br main issue is that, somehow quadratic and cubic interpolation results revert back to pre 0.19 behavior.

ev-br · 2017-03-16T15:10:04Z

Did they? That's bizzare. I can take a look --- a self-contained example would help though.

zym1010 · 2017-03-16T15:17:39Z

@ev-br

first one (copied from #15662), the expected result of scipy 0.19, is obtained using conda-forge built Scipy. This was tested on an unpatched pandas.

FAIL: test_interp_scipy_basic (pandas.tests.series.test_missing.TestSeriesInterpolateData)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/yimengzh/miniconda2/envs/default35/lib/python3.5/site-packages/pandas/tests/series/test_missing.py", line 711, in test_interp_scipy_basic
    assert_series_equal(result, expected)
  File "/Users/yimengzh/miniconda2/envs/default35/lib/python3.5/site-packages/pandas/util/testing.py", line 1181, in assert_series_equal
    obj='{0}'.format(obj))
  File "pandas/src/testing.pyx", line 59, in pandas._testing.assert_almost_equal (pandas/src/testing.c:4156)
  File "pandas/src/testing.pyx", line 173, in pandas._testing.assert_almost_equal (pandas/src/testing.c:3274)
  File "/Users/yimengzh/miniconda2/envs/default35/lib/python3.5/site-packages/pandas/util/testing.py", line 1018, in raise_assert_detail
    raise AssertionError(msg)
AssertionError: Series are different

Series values are different (33.33333 %)
[left]:  [1.0, 3.0, 6.82352941176, 12.0, 18.0588235294, 25.0]
[right]: [1.0, 3.0, 6.769231, 12.0, 18.230769, 25.0]

Then the one @jreback just discovered (copied from last part of https://travis-ci.org/pandas-dev/pandas/jobs/211720204), is probably obtained on dev version of scipy. This is tested on a patched pandas.

>       raise AssertionError(msg)
E       AssertionError: Series are different
E       
E       Series values are different (33.33333 %)
E       [left]:  [1.0, 3.0, 6.76923076923, 12.0, 18.2307692308, 25.0]
E       [right]: [1.0, 3.0, 6.823529, 12.0, 18.058824, 25.0]

Left result is always the result computed by scipy, and right result is the expected result hardcoded in test suite.

ev-br · 2017-03-16T15:29:49Z

probably obtained on dev version of scipy

Hmmm, can you cook up an example which just makes scipy calls? Will be easier for me to not have to navigate through the pandas codebase which I'm not familiar with.

zym1010 · 2017-03-16T15:31:43Z

Hmmm, can you cook up an example which just makes scipy calls? Will be easier for me to not have to navigate through the pandas codebase which I'm not familiar with.

@jreback can you help with this? As I'm not sure how you set up your scipy environments.

jreback · 2017-03-16T15:46:15Z

this is from defaults (numpy 1.12) channel. We are also installing 0.19.0 on conda-forge (both these match as below).

The fail is from

numpy: 1.13.0.dev0+dc04055
scipy: 0.19.0.dev0+ce24da1

# install numpy wheel from master
pip install --pre --upgrade --no-index --timeout=60 --trusted-host travis-dev-wheels.scipy.org -f http://travis-dev-wheels.scipy.org/ numpy scipy

In [9]: import scipy

In [10]: scipy.__version__
Out[10]: '0.19.0'

In [11]: from scipy import interpolate

In [12]: interpolate.interp1d(np.array([0, 1, 3, 5]), np.array([  1.,   3.,  12.,  25.]), kind='quadratic', fill_value=np.nan, bounds_error=False)(np.array([2, 4]))
Out[12]: array([  6.82352941,  18.05882353])

ev-br · 2017-03-16T15:50:53Z

Seems same for a -dev version:

In [4]: interpolate.interp1d(np.array([0, 1, 3, 5]), np.array([  1.,   3.,  12.,  25.]), kind='quadratic', fill_value=np.nan, bounds_error=False)(np.array([2, 4]))
Out[4]: array([  6.82352941,  18.05882353])

In [5]: import scipy

In [6]: scipy.__version__
Out[6]: '1.0.0.dev0+f3e4e17'

jreback · 2017-03-16T15:55:11Z

http://travis-dev-wheels.scipy.org/

hmm these are really old for scipy! are they being uploaded?

ev-br · 2017-03-16T16:01:29Z

Ah. Might be that scipy CI only uploads dev wheels in after_success (https://github.com/scipy/scipy/blob/master/.travis.yml#L142) and the builds with numpy-dev fail for the last couple of days because of a numpy-dev regression.

jreback · 2017-03-16T16:01:33Z

just going to remove it from that build: #15699

jreback · 2017-03-16T16:02:08Z

@ev-br hmm, there should be older ones though right? (well not that old).

ev-br · 2017-03-16T16:17:05Z

Indeed. scipy/scipy#7188

fix pandas-dev#15662 Author: Yimeng Zhang <[email protected]> Closes pandas-dev#15689 from zym1010/fix_scipy019 and squashes the following commits: 3cc6528 [Yimeng Zhang] doc and PEP8 9ed7524 [Yimeng Zhang] fix interpolation related issue with scipy 0.19 ca09705 [Yimeng Zhang] get symmetric window

get symmetric window

ca09705

stonebig mentioned this pull request Mar 14, 2017

release 2017-01 follow-up winpython/winpython#468

Closed

54 tasks

zym1010 mentioned this pull request Mar 14, 2017

Pandas test fails with Scipy 0.19 #15662

Closed

jreback approved these changes Mar 14, 2017

View reviewed changes

jreback added Compat pandas objects compatability with Numpy or Python functions Numeric Operations Arithmetic, Comparison, and Logical operations Testing pandas testing functions or related to the test suite labels Mar 14, 2017

fix interpolation related issue with scipy 0.19

9ed7524

jreback reviewed Mar 14, 2017

View reviewed changes

doc and PEP8

3cc6528

jreback added this to the 0.20.0 milestone Mar 15, 2017

jreback closed this in 76e5185 Mar 15, 2017

jreback mentioned this pull request Mar 16, 2017

TST: some scipy 0.19.0-dev tests failing #15696

Closed

ev-br reviewed Mar 16, 2017

View reviewed changes

ev-br mentioned this pull request Mar 16, 2017

Dev wheels not uploaded? scipy/scipy#7188

Closed

matthew-brett mentioned this pull request Mar 16, 2017

MAINT: test with manylinux numpy/scipy pre-release #15702

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compatibility with scipy 0.19 #15689

compatibility with scipy 0.19 #15689

zym1010 commented Mar 14, 2017 •

edited

Loading

jreback left a comment

jreback Mar 14, 2017

zym1010 Mar 14, 2017

jreback Mar 14, 2017

zym1010 Mar 14, 2017

jreback Mar 14, 2017

jreback commented Mar 14, 2017 •

edited

Loading

zym1010 commented Mar 14, 2017

jreback commented Mar 14, 2017

zym1010 commented Mar 14, 2017

jreback Mar 14, 2017

jreback Mar 14, 2017

jreback commented Mar 14, 2017

codecov bot commented Mar 15, 2017 •

edited

Loading

jreback commented Mar 15, 2017

jreback commented Mar 15, 2017

jreback commented Mar 16, 2017

jreback commented Mar 16, 2017

zym1010 commented Mar 16, 2017

zym1010 commented Mar 16, 2017

ev-br Mar 16, 2017 •

edited

Loading

zym1010 Mar 16, 2017

zym1010 Mar 16, 2017

ev-br commented Mar 16, 2017

zym1010 commented Mar 16, 2017

ev-br commented Mar 16, 2017

zym1010 commented Mar 16, 2017

jreback commented Mar 16, 2017 •

edited

Loading

ev-br commented Mar 16, 2017

jreback commented Mar 16, 2017

ev-br commented Mar 16, 2017

jreback commented Mar 16, 2017

jreback commented Mar 16, 2017

ev-br commented Mar 16, 2017

compatibility with scipy 0.19 #15689

compatibility with scipy 0.19 #15689

Conversation

zym1010 commented Mar 14, 2017 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Mar 14, 2017 • edited Loading

zym1010 commented Mar 14, 2017

jreback commented Mar 14, 2017

zym1010 commented Mar 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Mar 14, 2017

codecov bot commented Mar 15, 2017 • edited Loading

Codecov Report

jreback commented Mar 15, 2017

jreback commented Mar 15, 2017

jreback commented Mar 16, 2017

jreback commented Mar 16, 2017

zym1010 commented Mar 16, 2017

zym1010 commented Mar 16, 2017

ev-br Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ev-br commented Mar 16, 2017

zym1010 commented Mar 16, 2017

ev-br commented Mar 16, 2017

zym1010 commented Mar 16, 2017

jreback commented Mar 16, 2017 • edited Loading

ev-br commented Mar 16, 2017

jreback commented Mar 16, 2017

ev-br commented Mar 16, 2017

jreback commented Mar 16, 2017

jreback commented Mar 16, 2017

ev-br commented Mar 16, 2017

zym1010 commented Mar 14, 2017 •

edited

Loading

jreback commented Mar 14, 2017 •

edited

Loading

codecov bot commented Mar 15, 2017 •

edited

Loading

ev-br Mar 16, 2017 •

edited

Loading

jreback commented Mar 16, 2017 •

edited

Loading