Operator tests

[honno]

Should resolve #27, if partially. Commits start from #32.

Currently just parametrizes expressions to evaluate. As expected, testing the inplace operators require more thought.

[asmeurer]

We could test the operator methods like x1.__add__(x2) instead of x1 + x2. That may remove the need to do eval. In particular, we can use the operator module to treat x1 + x2 as the function operator.__add__.

It would also make it possible to test the right operators like __radd__, which aren't otherwise easy to test (although to be fair, this also implies that these methods aren't actually that important).

[honno]

(I've come up with a solution, so now there's full test coverage of the respective standard and inplace operators of the elementwise tests, as well as scenarios where scalars are in play... it's not worth reviewing yet though as things are still in flux)

• Test with both (Python scalar) integers and floats where appropriate bit tricky, prob something to leave for a future PR

[asmeurer]

Sorry I've fallen a bit behind in reviewing. Is this PR ready for review and merge?

[asmeurer]

Why was the broadcasting test moved to meta/? It seems to me that it's an actual test of array API module behavior, not a meta test.

[honno]

Sorry I've fallen a bit behind in reviewing. Is this PR ready for review and merge?

Yep, review ready, and potentially merge ready if you're happy with it.

Why was the broadcasting test moved to meta/? It seems to me that it's an actual test of array API module behavior, not a meta test.

Ah IIRC it was actually testing your custom implementation of the broadcasting algo from the spec, so I moved it there to clarify that. Oh maybe not, I may of accidentality moved test_broadcasting_hypothesis as well. I can go through the commit history tomorrow to see what was going on. The other tests are definitely meta tests.

[asmeurer]

I implemented the algorithm from the spec and use it as a baseline to test the array API library. I suppose we could split out the testing of the algorithm itself into meta tests, but the main point is to test the broadcasting behavior in the array library.

To be sure, this test is less important now that we actually have tests for the actual functions. So maybe it should be refactored. A line in each multiarg function test like assert res.shape == _broadcast_shapes(x1.shape, x2.shape) is basically equivalent. Unlike type promotion, there may not be much benefit to having it separate once the actual function tests exist. Perhaps this was your reasoning for moving things around?

[honno]

To be sure, this test is less important now that we actually have tests for the actual functions. So maybe it should be refactored. A line in each multiarg function test like assert res.shape == _broadcast_shapes(x1.shape, x2.shape) is basically equivalent. Unlike type promotion, there may not be much benefit to having it separate once the actual function tests exist. Perhaps this was your reasoning for moving things around?

It wasn't, I just made a mistake haha. But now that you mention that, we are indeed testing broadcasting for all the related functions (or should be), so I believe we don't need it anymore. I've just removed the method. Whilst parametrized tests for broadcasting is nice, I think it's the right decision not to specifically test broadcasting (just like we removed those complicated test_type_promotion.py tests). You think that's okay?

[asmeurer]

Yes, this sounds good. In the future, we might have to think about if we need to split tests up to help implementers who may want to test certain things even when other things aren't working. But that's something we should think about only once it becomes an issue. We may find that the most practical method for implementers will be to just directly comment out broken bits to see if other parts work. For now the test suite is heavily biased toward conforming or nearly conforming libraries, and it will be difficult to make it otherwise without some specific example to work against.

[asmeurer]

Merging this so that the other PR based on it is easier to review.