Repository navigation
Fix binding logic in @overload_glue. - #7996
Conversation
Updates the binding logic to accommodate calling `@overload_glue` wrapped functions with named arguments. Fixes numba#7982
377b3ff to
0e5aece
Compare
@overload_glue.
| func_name = self.key.__name__ | ||
|
|
||
| def typer(ptr, shape, dtype=types.none): | ||
| def typer(ptr, shape, dtype=None): |
There was a problem hiding this comment.
This change needs some thinking about. I think the default now matches what an @overload would implement, but I'm not sure if this is correct WRT @overload_glue. There's also the question of whether the subsequent type check needs to be a is_nonelike() type check for the case of a types.none appearing, not sure if this is actually possible yet.
There was a problem hiding this comment.
Hm... that's a problem. It will be a types.none when called with c_arr = numba.carray(arr, (3,), dtype=None).
There was a problem hiding this comment.
The following will fail:
import ctypes
import numba
import numpy as np
@numba.cfunc(numba.types.void(
numba.types.CPointer(numba.types.double)
))
def func(arr: np.ndarray) -> None:
c_arr = numba.carray(arr, (3,), dtype=None)
c_arr[-1] = 1
my_arr = np.array([1, 2, 3], dtype=np.float_).ctypes.data_as(ctypes.POINTER(ctypes.c_double))
func(my_arr)The OverloadSelector cannot find the impl:
File "/path/to/numba/numba/core/base.py", line 44, in find
out = self._find(sig)
File "/path/to/numba/numba/core/base.py", line 53, in _find
raise errors.NumbaNotImplementedError(f'{self}, {sig}')
numba.core.errors.NumbaNotImplementedError: <numba.core.base.OverloadSelector object at 0x7ff7d85c3310>, (float64*, UniTuple(Literal[int](3) x 1), none)
There was a problem hiding this comment.
From OOB chat, time would be better spent translating this to @overload than trying to fix this edge case in @overload_glue, added this as issue: #8009
sklam
left a comment
There was a problem hiding this comment.
The patch looks good in general. Just one nitpick on the change to use generator for the varnames. Why not just a simple sequence (list/tuple)?
As title.
| """This generates a function based on the argnames generated by the | ||
| varnames_gen generator, the "body_func" is the function that'll |
There was a problem hiding this comment.
| """This generates a function based on the argnames generated by the | |
| varnames_gen generator, the "body_func" is the function that'll | |
| """This generates a function based on the argnames provided by | |
| varnames, the "body_func" is the function that'll |
There was a problem hiding this comment.
Thanks, fixed these and the side-effects of fixing these in 2e833f7
Rename variable and fix code/docstring.
Updates the binding logic to accommodate calling
@overload_gluewrapped functions with named arguments.
Fixes #7982