ENH: Added test cases to ensure the behavior of integer as it is. #3592

Merged
merged 4 commits into from Aug 23, 2013

Conversation

Projects
None yet
2 participants
Contributor

arinkverma commented Aug 8, 2013

There is a need to add test case which ensure and maintain the behavior of integer. As per pr #3567, which speed up integer scalar's operations by avoiding the conversion of integer to NumPy Scalar.

Test case

For np.int16

a = np.array([2**15-1]], dtype=np.int16)
b = np.array([-2**15], dtype=np.int16)           
assert_equal(a,b-1)

For np.uint16

a = np.array([2**16-1]], dtype=np.uint16)
b = np.array([0], dtype=np.uint16)           
assert_equal(a+1,b)
@arinkverma arinkverma ENH: Added test cases to ensure the behavior of integer as it is.
There is a need to add test case which ensure and maintain the behaviour
of integer. As per pr #3567, which speed up integer scalar's operations
by avoiding the conversion of integer to NumPy Scalar.
6ee3aba

@charris charris commented on an outdated diff Aug 10, 2013

numpy/core/tests/test_scalarmath.py
+
+ l = [0, 2**8-1, 2**16-1, 2**32-1, 2**64-1]
+ li = [1, 2**8, 2**16, 2**32, 2**64]
+ x = 1
+ for T in [np.uint8, np.uint16, np.uint32, np.uint64]:
+ a1 = np.array([l[:x]],dtype=T)
+ b1= np.array([li[:x]], dtype=T)
+ assert_equal(a1+1,b1)
+
+ a2 = np.array([l[x]],dtype=T)
+ b2= np.array([0], dtype=T)
+ assert_equal(a2+1,b2)
+ x = x+1
+
+
+ def test_long_os_behaviour(self):
@charris

charris Aug 10, 2013

Owner

What this is testing is iinfo, not long, so test_iinfo_long_values would be more appropriate, with separate tests for signed and unsigned.

@charris

charris Aug 10, 2013

Owner

That said, I'm not sure that it is easy to be sure of the validity of these tests on all platforms...

@charris charris commented on an outdated diff Aug 10, 2013

numpy/core/tests/test_scalarmath.py
+ l = [0, 2**8-1, 2**16-1, 2**32-1, 2**64-1]
+ li = [1, 2**8, 2**16, 2**32, 2**64]
+ x = 1
+ for T in [np.uint8, np.uint16, np.uint32, np.uint64]:
+ a1 = np.array([l[:x]],dtype=T)
+ b1= np.array([li[:x]], dtype=T)
+ assert_equal(a1+1,b1)
+
+ a2 = np.array([l[x]],dtype=T)
+ b2= np.array([0], dtype=T)
+ assert_equal(a2+1,b2)
+ x = x+1
+
+
+ def test_long_os_behaviour(self):
+ Long = np.iinfo('l')
@charris

charris Aug 10, 2013

Owner

Numpy only uses caps for classes. Best use something like long_ here.

@charris charris commented on an outdated diff Aug 10, 2013

numpy/core/tests/test_scalarmath.py
@@ -139,6 +140,54 @@ def test_int_from_long(self):
assert_equal([int(_m) for _m in a], li[:3])
+ def test_int_value_behaviour(self):
+ l = [0, 2**7-1, 2**15-1, 2**31-1, 2**63-1]
+ li = [-1, -2**7, -2**15, -2**31, -2**63]
+ x = 1
+ for T in [np.int8, np.int16, np.int32, np.int64]:
@charris

charris Aug 10, 2013

Owner

dt instead of T would avoid the capital.

@charris

charris Aug 10, 2013

Owner

I think for your work you want to test the C types rather than the bit specified types, so something like

for code in 'bhilq':

will get you char, short, int, long, and long long. 'BHILQ' will get you the corresponding unsigned types.

@charris charris commented on an outdated diff Aug 10, 2013

numpy/core/tests/test_scalarmath.py
@@ -139,6 +140,54 @@ def test_int_from_long(self):
assert_equal([int(_m) for _m in a], li[:3])
+ def test_int_value_behaviour(self):
+ l = [0, 2**7-1, 2**15-1, 2**31-1, 2**63-1]
+ li = [-1, -2**7, -2**15, -2**31, -2**63]
+ x = 1
+ for T in [np.int8, np.int16, np.int32, np.int64]:
+ a1 = np.array([l[:x]],dtype=T)
+ b1= np.array([li[:x]], dtype=T)
+ assert_equal(-a1-1,b1)
+
+ a2 = np.array([l[x]],dtype=T)
+ b2= np.array([li[x]], dtype=T)
+ assert_equal(a2,b2-1)
+ x = x+1
@charris

charris Aug 10, 2013

Owner

For this sort of thing do

for x, code in enumerate('bhilq'):

That will get you the count starting at x = 0.

@charris charris commented on an outdated diff Aug 10, 2013

numpy/core/tests/test_scalarmath.py
@@ -139,6 +140,54 @@ def test_int_from_long(self):
assert_equal([int(_m) for _m in a], li[:3])
+ def test_int_value_behaviour(self):
+ l = [0, 2**7-1, 2**15-1, 2**31-1, 2**63-1]
+ li = [-1, -2**7, -2**15, -2**31, -2**63]
+ x = 1
+ for T in [np.int8, np.int16, np.int32, np.int64]:
+ a1 = np.array([l[:x]],dtype=T)
+ b1= np.array([li[:x]], dtype=T)
+ assert_equal(-a1-1,b1)
@charris

charris Aug 10, 2013

Owner

What you are testing here is arithmetic. To check that the conversion of the values wraps, do something like

for code in 'bBhH':
    res = array(iinfo(code).max + 1, dtype=code)
    tgt = iinfo(code).min
    assert_(res == tgt)

For if 'iI' have less precision than 'lL', this should work for them also. For a plain old test of conversion, you can do

for code in typecodes['AllInteger']:
    res = array(iinfo(code).max, dtype=code)
    tgt = iinfo(code).max
    assert_(res == tgt)

You can also just test the scalar types

for code in typecodes['AllInteger']:
    res = typeDict[code](iinfo(code).max)
    tgt = iinfo(code).max
    assert_(res == tgt)

Checking the errors requires using numbers bigger than the c long type for long

In [24]: for code in ['lLqQ']:
    res = typeDict[code](iinfo(code).max + 1)
   ....: 
---------------------------------------------------------------------------
OverflowError                             Traceback (most recent call last)
/home/charris/<ipython-input-24-d96c0443d98a> in <module>()
      1 for code in ['l']:
----> 2     res = typeDict[code](iinfo(code).max + 1)

OverflowError: Python int too large to convert to C long

This should also fail for 'iI' types if long is the same size as int, something that can be checked using the max value from iinfo.

@charris charris commented on an outdated diff Aug 10, 2013

numpy/core/tests/test_scalarmath.py
@@ -139,6 +140,54 @@ def test_int_from_long(self):
assert_equal([int(_m) for _m in a], li[:3])
+ def test_int_value_behaviour(self):
+ l = [0, 2**7-1, 2**15-1, 2**31-1, 2**63-1]
+ li = [-1, -2**7, -2**15, -2**31, -2**63]
+ x = 1
+ for T in [np.int8, np.int16, np.int32, np.int64]:
+ a1 = np.array([l[:x]],dtype=T)
+ b1= np.array([li[:x]], dtype=T)
+ assert_equal(-a1-1,b1)
+
+ a2 = np.array([l[x]],dtype=T)
@charris

charris Aug 10, 2013

Owner

Be careful about the spacing after commas and around + and ==

@charris charris commented on an outdated diff Aug 17, 2013

numpy/core/tests/test_scalarmath.py
+ tgt = np.iinfo(code).max
+ assert_(res == tgt)
+
+ for code in np.typecodes['AllInteger']:
+ res = np.typeDict[code](np.iinfo(code).max)
+ tgt = np.iinfo(code).max
+ assert_(res == tgt)
+
+
+ def test_int_raise_behaviour(self):
+
+ def Overflow_error_func(dtype):
+ res = np.typeDict[dtype](np.iinfo(dtype).max + 1)
+
+ for code in 'lLqQ':
+ assert_raises(OverflowError,Overflow_error_func,code)
@charris

charris Aug 17, 2013

Owner

Spaces after ,.

@charris charris commented on an outdated diff Aug 17, 2013

numpy/core/tests/test_scalarmath.py
+ for code in np.typecodes['AllInteger']:
+ res = np.typeDict[code](np.iinfo(code).max)
+ tgt = np.iinfo(code).max
+ assert_(res == tgt)
+
+
+ def test_int_raise_behaviour(self):
+
+ def Overflow_error_func(dtype):
+ res = np.typeDict[dtype](np.iinfo(dtype).max + 1)
+
+ for code in 'lLqQ':
+ assert_raises(OverflowError,Overflow_error_func,code)
+
+
+ def test_long_os_behaviour(self):
@charris

charris Aug 17, 2013

Owner

Because we can't know which choices all potential platforms make, I think it best to omit this test. The configuration run before compilation should set these correctly.

@charris charris commented on an outdated diff Aug 17, 2013

numpy/core/tests/test_scalarmath.py
+ assert_(res == tgt)
+
+
+ def test_int_raise_behaviour(self):
+
+ def Overflow_error_func(dtype):
+ res = np.typeDict[dtype](np.iinfo(dtype).max + 1)
+
+ for code in 'lLqQ':
+ assert_raises(OverflowError,Overflow_error_func,code)
+
+
+ def test_long_os_behaviour(self):
+ long_iinfo = np.iinfo('l')
+ ulong_iinfo = np.iinfo('L')
+ if sys.platform == "win32" or sys.platform == "win64" \
@charris

charris Aug 17, 2013

Owner

For future reference, instead of \, enclose the logic in (...), see http://www.python.org/dev/peps/pep-0008/#maximum-line-length. The lines in the continuation should be indented so they cannot be confused with the body of the if statement.

Owner

charris commented Aug 17, 2013

Looks good modulo some nitpicks.

Owner

charris commented Aug 17, 2013

When pushing updates to a PR, it is best to make a comment afterward, otherwise github won't send any notification.

@charles, so there is no need of test cases for different os..

@charris charris commented on an outdated diff Aug 20, 2013

numpy/core/tests/test_scalarmath.py
+ for code in np.typecodes['AllInteger']:
+ res = np.typeDict[code](np.iinfo(code).max)
+ tgt = np.iinfo(code).max
+ assert_(res == tgt)
+
+
+ def test_int_raise_behaviour(self):
+
+ def Overflow_error_func(dtype):
+ res = np.typeDict[dtype](np.iinfo(dtype).max + 1)
+
+ for code in 'lLqQ':
+ assert_raises(OverflowError, Overflow_error_func, code)
+
+
+ def test_long_os_behaviour(self):
@charris

charris Aug 20, 2013

Owner

This test should be removed, it is likely to be fragile. We don't want to test the OS choices on long conventions, we determine that during configuation in the build process.

Owner

charris commented Aug 23, 2013

OK, thanks. It is best to comment after a push as otherwise github won't send a notification out.

charris merged commit 266a1d1 into numpy:master Aug 23, 2013

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment