-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
array api-related upstream-dev failures #8854
Conversation
This would make it a dependency of `namedarray`, and not allow behavior that is allowed but not required by the array API standard. Otherwise we can: - use the main `numpy` namespace - use `array_api_compat` (would also be a new dependency) to allow optional behavior
My intention with the tests in namedarray were intended to be using the strict array, so I think It should not be required for anything else than testing though.
These should probably just be |
my question was about the import of Edit: I'll apply your suggestion tomorrow |
Ah the doctests, the idea was to use an array api compliant array in the array api module. Previously numpy couldn't sometimes pass through the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should probably just be
nxp.asarray([2.1, 4], dtype=nxp.int64)
?
that would have worked if np.array
reprs would include the default dtype, so we might need to use a different dtype (int16
or int8
, maybe? int32
has the same issue on a 32-bit OS)
In any case I'm unlikely to be able to continue working on this until the weekend next week, so if anyone wants to pick this up feel free to!
unless I missed any (which is definitely possible), I've got it down to 3 unique errors:
Edit: I'd appreciate input on how to best resolve 1 and 2 |
For 2 you can calculate bytes from bits with |
This is not explicitly allowed by the array API specification (it was declared out of scope), so I'm not sure if this is the right way to do this.
that works, although for floating dtypes there's |
yeah, here's my version I was playing around with: import array_api_strict as xp
a = xp.asarray([1, 2, 3], dtype=xp.int8)
def itemsize(x) -> int:
"""
Length of one array element in bytes.
Parameters
----------
x :
array to get the itemsize of.
Examples
--------
>>> import array_api_strict as xp
Bool
>>> x = xp.asarray([0,1,2], dtype=xp.bool)
>>> itemsize(xp.astype(x, xp.bool))
1
Integer
>>> itemsize(xp.astype(x, xp.int8))
1
>>> itemsize(xp.astype(x, xp.int16))
2
>>> itemsize(xp.astype(x, xp.int32))
4
>>> itemsize(xp.astype(x, xp.int64))
8
Floating
>>> itemsize(xp.astype(x, xp.float32))
4
>>> itemsize(xp.astype(x, xp.float64))
8
Floating complex
>>> itemsize(xp.astype(x, xp.complex64))
4
>>> itemsize(xp.astype(x, xp.complex128))
8
"""
xp = x.__array_namespace__() if hasattr(x, "__array_namespace__") else np
dtype = x.dtype
if xp.isdtype(dtype, kind="bool"):
return 1
elif xp.isdtype(dtype, kind="integral"):
return xp.iinfo(dtype).bits // 8
else:
return xp.finfo(dtype).bits // 8 To get xarray/xarray/namedarray/_array_api.py Line 33 in 1d43672
|
`xp.isdtype` should already check the same thing.
@shoyer, could I ask for another review? I think I changed the code to what we discussed yesterday, but I might have missed something. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you!
I made a few suggestions of code comments for future readers.
Co-authored-by: Stephan Hoyer <[email protected]>
for more information, see https://pre-commit.ci
the remaining two errors on the upstream-dev CI are due to a bug in |
looks like this should finally be ready for merging? |
This "fixes" the upstream-dev failures related to the removal of
numpy.array_api
. There are a couple of open questions, though:array-api-strict
is not installed by default, sonamedarray
would get a new dependency. Not sure how to deal with that – as far as I can tell,numpy.array_api
was not supposed to be used that way, so maybe we need to usearray-api-compat
instead? What do you think, @andersy005, @Illviljan?array-api-strict
does not defineArray.nbytes
(causing a funny exception that wrongly claimsDataArray
does not definenbytes
)array-api-strict
has a differentDType
class, which makes it tricky to work with bothnumpy
dtypes and said dtype class in the same code. In particular, if I understand correctly we're supposed to check dtypes usingisdtype
, butnumpy.isdtype
will only exist innumpy>=2
,array-api-strict
's version does not define datetime / string / object dtypes, andnumpy.issubdtype
does not work with the non-numpy
dtype class). So maybe we need to usearray-api-compat
internally?