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

BLD: numpy master changes breaking #12049

Closed
jreback opened this issue Jan 15, 2016 · 9 comments · Fixed by #12058
Closed

BLD: numpy master changes breaking #12049

jreback opened this issue Jan 15, 2016 · 9 comments · Fixed by #12058
Labels
Build Library building on various platforms Compat pandas objects compatability with Numpy or Python functions
Milestone

Comments

@jreback
Copy link
Contributor

jreback commented Jan 15, 2016

(24 hrs ago) good build: https://travis-ci.org/pydata/pandas/jobs/102356098: 1.11.0.dev0+51d2ecd
(1 hr ago) breaking lots of things: https://travis-ci.org/pydata/pandas/jobs/102596904: 1.11.0.dev0+aa6335c

@shoyer IIRC a couple of your PR's were merged in the last day.

here's an example:

======================================================================
FAIL: test_coercion_with_setitem_and_series (pandas.tests.test_indexing.TestSeriesNoneCoercion)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/pydata/pandas/pandas/tests/test_indexing.py", line 5247, in test_coercion_with_setitem_and_series
    expected_series.values, strict_nan=True)
  File "/home/travis/build/pydata/pandas/pandas/util/testing.py", line 866, in assert_numpy_array_equal
    raise_assert_detail(obj, msg, left, right)
  File "/home/travis/build/pydata/pandas/pandas/util/testing.py", line 825, in raise_assert_detail
    raise AssertionError(msg)
AssertionError: numpy array are different
numpy array values are different (33.33333 %)
[left]:  [NaT, 2000-01-02T00:00:00.000000000+0000, 2000-01-03T00:00:00.000000000+0000]
[right]: [NaT, 2000-01-02T00:00:00.000000000+0000, 2000-01-03T00:00:00.000000000+0000]
@jreback jreback added Build Library building on various platforms Compat pandas objects compatability with Numpy or Python functions labels Jan 15, 2016
@jreback jreback added this to the 0.18.0 milestone Jan 15, 2016
@jreback
Copy link
Contributor Author

jreback commented Jan 15, 2016

my guess is that we now need to compare M8[ns] by .view('i8') then comparing as the NaT will then compare equal.

@shoyer
Copy link
Member

shoyer commented Jan 15, 2016

Yep, if this is datetime64 related it's my fault :). I though we were already using .view('i8') before making comparisons but I guess I was wrong. If so (especially if this breaks user facing stuff) we may need to hold off on these numpy fixes for a little longer (deprecation cycle?). Sigh...

On Fri, Jan 15, 2016 at 7:27 AM, Jeff Reback notifications@github.com
wrote:

my guess is that we now need to compare M8[ns] by .view('i8') then comparing as the NaT will then compare equal.

Reply to this email directly or view it on GitHub:
#12049 (comment)

@jreback
Copy link
Contributor Author

jreback commented Jan 15, 2016

we can adapt. do you want me to open an issue on numpy?

In [1]: np.__version__
Out[1]: '1.10.2'

In [5]: arr = np.array([np.nan])

In [6]: np.array_equal(arr,arr)
Out[6]: False

In [7]: arr = np.array([np.datetime64('NaT')])

In [8]: np.array_equal(arr,arr)
Out[8]: True

ok so your change makes sense

In [1]: np.__version__
Out[1]: '1.11.0.dev0+aa6335c'

In [2]: In [5]: arr = np.array([np.nan])

In [7]: arr = np.array([np.nan])

In [8]: np.array_equal(arr,arr)
Out[8]: False

In [9]: arr = np.array([np.datetime64('NaT')])

In [10]: np.array_equal(arr,arr)
Out[10]: False

@shoyer
Copy link
Member

shoyer commented Jan 15, 2016

Actually, looks like we might just be able to drop our assert_numpy_array_equivalent entirely in favor of numpy's assert_array_equal, which has supported NaN equality since at least August 2011 (numpy 1.7, I think):
numpy/numpy@67ece6b

@shoyer
Copy link
Member

shoyer commented Jan 15, 2016

Unfortunately, this bug exists in our array_equivalent utility function, which we use for the equals method. This means that with this change datetime64 equality checks involving NaT will be broken. As much as I would love to just roll out the NumPy fix, doing it like this will assuredly result in unhappy users and unnecessary aggravation when sanity checks and test suites fail.

To fix this, I propose:

  • We roll back the NaT comparison fix in NumPy for now, issuing a deprecation warning instead for a numpy release or two.
  • We add the fix to array_equivalent in pandas, doing the appropriate cast to int64 to avoid needing to catch the deprecation warning (which can have performance consequences).

@jreback
Copy link
Contributor Author

jreback commented Jan 15, 2016

yep, prob need a special case for M8/m8 (alternatively we could have a conditional check on the numpy version).

Ok, lmk if you need anything on the numpy side.

@njsmith
Copy link

njsmith commented Jan 15, 2016

BTW, numpy's travis jobs now continuously upload pre-built (as wheels) snapshots of numpy master to http://travis-dev-wheels.scipy.org/ -- might simplify your test infrastructure for testing against numpy master. (Looks like we're only doing this for py35 right now, but it'd be easy to add more builds if you need them.)

@njsmith
Copy link

njsmith commented Jan 15, 2016

It looks like the magic incantation is:

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

@jreback
Copy link
Contributor Author

jreback commented Jan 15, 2016

@njsmith ok, thanks!
yeh that would be ideal to have 2.7/3.5 wheels (right now we are just testing vs 2.7 numpy master)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms Compat pandas objects compatability with Numpy or Python functions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants