ENH: disallow passing array-like to array_namespace #42
Conversation
|
Thanks. It looks like the initial reason for me doing this isn't even necessary anymore. I don't call array_namespace in wrapped functions. The only exception is in the internal |
Oh, I am just seeing this function. |
|
Oh I hadn't noticed that your function is basically the same thing. Yeah I think we can add it publicly here if that helps. If we do that, I wonder if we should rename it. If we didn't, we would have different
So maybe it would be less confusing to call it I'm also unsure about the |
Would be great! I think we have 2 ways moving forward: either we wait to see how people use this library and what sort of wrapper/helper they need, or make a round table now to discuss of our needs and propose the wrappers. I have a preference for both as it would simplify/slim future maintenance work. But since all of that is private, we can also wait and do our cooking in the background.
I would not think this is an issue. It's quite clear if you conserve the namespace in the function call. Of course if you start to import everything with
For the SciPy PR I have this but it does a combination of
This can be useful if you have a simple function which just deal with the array. In this case you don't need to know the namespace (no need to call before |
We should add anything that's generally useful to multiple libraries (as long as it keeps the compat library itself simple, i.e., pure Python code only). The goal of the compat library is to make adoption as easy as possible, so these little helper tools are definitely in scope for it. That means that if a need comes up we can add things, but if we see something ahead of time that could be useful we can add it right away.
Not just import * but even just We could maybe avoid this if we make
OK that seems fair enough, as long as we document the behavior. It is important, though, to make sure you do use |
|
Is there anything else to do in this PR? |
|
About |
|
What should we do with this PR? I know you ended up adding an |
|
mmm we are having 2 discussions here. This PR was just about disallowing array-like to For the |
So I think this PR should be merged, since passing lists or tuples of compatible arrays to functions expecting a single array is weird.
def _check_finite(array, xp):
"""Check for NaNs or Infs."""
msg = "array must not contain infs or NaNs"
try:
if not xp.all(xp.isfinite(array)):
raise ValueError(msg)
except TypeError:
raise ValueError(msg)
def as_xparray(
array, dtype=None, order=None, copy=None, *, xp=None, check_finite=False
):
"""SciPy-specific replacement for `np.asarray` with `order` and `check_finite`.
Memory layout parameter `order` is not exposed in the Array API standard.
`order` is only enforced if the input array implementation
is NumPy based, otherwise `order` is just silently ignored.
`check_finite` is also not a keyword in the array API standard; included
here for convenience rather than that having to be a separate function
call inside SciPy functions.
"""
if xp is None:
xp = array_namespace(array)
if xp.__name__ in {"numpy", "scipy._lib.array_api_compat.array_api_compat.numpy"}:
# Use NumPy API to support order
if copy is True:
array = np.array(array, order=order, dtype=dtype)
else:
array = np.asarray(array, order=order, dtype=dtype)
# At this point array is a NumPy ndarray. We convert it to an array
# container that is consistent with the input's namespace.
array = xp.asarray(array)
else:
try:
array = xp.asarray(array, dtype=dtype, copy=copy)
except TypeError:
coerced_xp = array_namespace(xp.asarray(3))
array = coerced_xp.asarray(array, dtype=dtype, copy=copy)
if check_finite:
_check_finite(array, xp)
return array
Your call @asmeurer |
Following discussions in #39