Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for array arguments in the finfo and iinfo functions across multiple backend modules while ensuring compatibility with array API tests. Key changes include wrapping finfo and iinfo calls using the get_xp function in torch, numpy, cupy, and dask modules, and defining common implementations in the shared module.
Reviewed Changes
Copilot reviewed 5 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| array_api_compat/torch/_aliases.py | Added finfo and iinfo wrappers and updated all for torch backend |
| array_api_compat/numpy/_aliases.py | Added finfo and iinfo wrappers for numpy backend |
| array_api_compat/dask/array/_aliases.py | Introduced finfo and iinfo wrappers; all no longer exports them |
| array_api_compat/cupy/_aliases.py | Added finfo and iinfo wrappers for cupy backend |
| array_api_compat/common/_aliases.py | Implemented common finfo and iinfo definitions and updated all accordingly |
Files not reviewed (7)
- cupy-xfails.txt: Language not supported
- dask-xfails.txt: Language not supported
- numpy-1-21-xfails.txt: Language not supported
- numpy-1-26-xfails.txt: Language not supported
- numpy-dev-xfails.txt: Language not supported
- numpy-xfails.txt: Language not supported
- torch-xfails.txt: Language not supported
Comments suppressed due to low confidence (1)
array_api_compat/dask/array/_aliases.py:346
- The public API (all) in the dask module no longer exports 'finfo' and 'iinfo', which is inconsistent with other backends. Consider adding 'finfo' and 'iinfo' back to all to maintain a consistent interface.
'bitwise_right_shift', 'concat', 'pow', 'can_cast',
cupy-xfails.txt
Outdated
| array_api_tests/test_data_type_functions.py::test_finfo[float32] | ||
| # attributes are np.float32 instead of float | ||
| # (see also https://github.com/data-apis/array-api/issues/405) | ||
| array_api_tests/test_data_type_functions.py::test_finfo[float32-dtype] |
There was a problem hiding this comment.
alternatively to this mess, considering the ongoing controversy in data-apis/array-api#405, we could relax array-api-tests to check that the output is either a float or a 0-dimensional real floating array.
lucascolley
left a comment
There was a problem hiding this comment.
these changes LGTM, but the CI failure looks related
Yes, the CI failure needs data-apis/array-api-tests#358
So possibly the best plan is to
|
|
I've updated both this PR and the test in array-api-strict to make merging easier. CI should be green now (but the new feature this PR introduces won't be tested until the array-api-tests PR gets merged). |
|
Okay, let's untie this tangle. Changes LGTM, let's land them now. |
iinfo/finfoarray-api-tests#358