Repository navigation
Signature tests not catching missing keyword arguments #175
Description
For array_api_compat.torch, the triu function is missing the k keyword argument. But none of the tests catch this:
array_api_tests/test_has_names.py::test_has_names[creation-triu] PASSED [ 50%]
array_api_tests/test_signatures.py::test_func_signature[triu] SKIPPED (Signature for triu() is not inspectable and is too troublesome to test for otherwise) [100%]
This is a great example of something that the old signature tests would handle just fine. I just tested it in 2022.03.22, the version before #110 was merged, and it finds the error just fine.
E AssertionError: Unexpected exception TypeError("triu() got an unexpected keyword argument 'k'"): triu() should accept the keyword-only argument 'k'
So let's please just reintroduce these old signature tests as a fallback. They are much better than nothing and hardly "too troublesome". We are missing real bugs because of this.
Metadata
Metadata
Assignees
Labels
No labels
Activity
asmeurer commentedon Mar 17, 2023
And this is at least the second instance of this data-apis/array-api-compat#22. Who knows how many other such issues there are.
honno commentedon Mar 28, 2023
To clarify, it's too troublesome to test with the same guarantees when testing inspectable signatures (e.g. trying different combinations of pos and/or keyword arguments for every arg that an Array API function supports).
Fallbacks are understandable but its weird to maintain given we (ideally) have primary tests for every function that implicitly check signatures anyway... but if they're not all getting implemented (like
triu()1) then lets go for it. I think I'd like a separate test case (i.e.test_signature_basic) to really hammer home the lack of thoroughness on our part, and maybe even only for the known uninspectable functions in the ecosystem (read: intorch).Footnotes
I didn't even know we had
triu()in2021.12haha, must of missed it when I was (evidently not) implementing the remaining creation tests way back when. ↩