Distinguish between np type hints from Py ones #4953
Conversation
jmao-denver
added
NoDocumentationNeeded
python-server-side
NoReleaseNotesNeeded
| return type(x) == {p_type} | ||
| """ | ||
| exec(func_str, globals()) | ||
| t = empty_table(1).update(["X = i", f"Y = f((float)X)"]) |
There was a problem hiding this comment.
This looks like it is only testing the np.float32 case or is casting.
There was a problem hiding this comment.
These tests are just to make sure that the converted args' types are what we expected, the exact types of the values are not a concern here and they are tested in other test cases in the same test class.
| return type(x) == {p_type} | ||
| """ | ||
| exec(func_str, globals()) | ||
| t = empty_table(1).update(["X = i", f"Y = f(({p_type})X)"]) |
There was a problem hiding this comment.
Seems like this should also be testing with long and double since the python types are wide.
There was a problem hiding this comment.
see the comment above.
| return type(x) == {p_type} | ||
| """ | ||
| exec(func_str, globals()) | ||
| t = empty_table(1).update(["X = i", f"Y = f(X)"]) |
There was a problem hiding this comment.
This looks like it is only testing the np.int32 case or is doing casting.
There was a problem hiding this comment.
see the comment above.
There was a problem hiding this comment.
I don't think the behavior here is correct. X is an np.int32. Here np.int8 is an accepted input in one iteration, which would be a downconversion -- certainly bad and should fail.
There was a problem hiding this comment.
We don't yet statically check if the parameter types and argument types match. That's something for the improvement of the QLP. Runtime exception however will be thrown if the arg value is outside the range of a numpy integer type.
Fixes #4933