Tests for argument conversion of Python bindings generator#15955
Tests for argument conversion of Python bindings generator#15955alalek merged 2 commits intoopencv:3.4from
Conversation
|
@alalek I've applied all your review comments and added new conversion tests. |
|
@alalek I resolved all issues with the MacOS build, can you proceed with review? |
modules/python/test/test_misc.py
Outdated
|
|
||
| def test_parse_to_int(self): | ||
| min_int, max_int = get_limits(ctypes.c_int) | ||
| for convertible in (-10, -1, 2, True, False, int(43.2), np.uint8(15), |
There was a problem hiding this comment.
True, False
Not sure that we really want them in this list (at least for type-safety and better overloads handling - we have bool+int parameters in a row).
/cc @mshabunin
There was a problem hiding this comment.
I added boolean to convertible list to state implicit conversion behavior.
I think we should leave overload resolution to C++ backend. For example, converter will indicate if such conversion occurred.
modules/python/test/test_misc.py
Outdated
|
|
||
| def test_parse_to_float(self): | ||
| min_float, max_float = get_limits(ctypes.c_float) | ||
| for convertible in (2, True, False, -13, 1.24, float(32), |
There was a problem hiding this comment.
float
True, False
IMHO, should move to non_convertible list.
| ) | ||
|
|
||
| min_double, max_double = get_limits(ctypes.c_double) | ||
| for inf in (min_float * 10, max_float * 10, min_double, max_double): |
There was a problem hiding this comment.
To check that if mantissa exceeds max value that can be held by float type it becomes inf.
Maybe this check is unnecessary, because max/min _double check the same case.
modules/python/test/test_misc.py
Outdated
| def test_parse_to_double(self): | ||
| min_float, max_float = get_limits(ctypes.c_float) | ||
| min_double, max_double = get_limits(ctypes.c_double) | ||
| for convertible in (2, True, False, -13, 1.24, np.float(32.45), |
modules/python/test/test_misc.py
Outdated
| self.assertEqual(res4, "InputArrayOfArrays: empty()=false kind=0x00050000 flags=0x01050000 total(-1)=3 dims(-1)=1 size(-1)=3x1 type(0)=CV_32FC2 dims(0)=2 size(0)=3x1 type(0)=CV_32FC2") | ||
|
|
||
| def test_parse_to_bool(self): | ||
| def try_to_convert(value): |
There was a problem hiding this comment.
As I can see all try_to_convert functions are the same, can we make a single method which will accept function to be called?
def try_to_convert(self, fun, value):
try:
result = fun(value).lower()
except Exception as e:
...
else:
return resultThere was a problem hiding this comment.
There is also "expected" part (+workarounds for Win32), so they are not the same.
modules/python/test/test_misc.py
Outdated
| )) | ||
|
|
||
| a = np.array([1, 2, 0], dtype=np.bool) | ||
| for i in range(len(a)): |
There was a problem hiding this comment.
Is it necessary to check each value in the array, can it be moved to previous blocks?
for convertible_true in (..., np.array([1, 2, 0], dtype=np.bool)[1]):
...
for convertible_false in (..., np.array([1, 2, 0], dtype=np.bool)[2]):Same comment for all following test cases.
There was a problem hiding this comment.
If this check will be moved to "convertible" block, it won't be possible to determine if conversion error occurs when array was indexed or just normal check.
There was a problem hiding this comment.
It can be determined from current iteration index.
There was a problem hiding this comment.
Hmm, I think we can completely eliminate this check if we add array element type to appropriate list. E.G:
type(np.array([1, 2], dtype=np.bool)[0]) # yields to np.bool_
type(np.array([1, 2], dtype=ctypes.c_size_t)[0]) # yields to np.uint64
type(np.array(['t1', 't2'])[0]) # yields to np.str_ which is np.strb2a4819 to
6f4836b
Compare
- Add positive and negative tests for int, float, double, size_t,
const char*, bool.
- Tests with wrong conversion behavior are skipped.
6f4836b to
0a4fb72
Compare
conversion behavior.
Added tests for Python bindings generated for functions with elemental types as input arguments
Should be treated as a part of tests for 15915
Bad cases, those are resolved by 15915 are added as skipped tests and should be enabled in the following PR.