-
Notifications
You must be signed in to change notification settings - Fork 45
Operator tests #35
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
Operator tests #35
Conversation
We could test the operator methods like It would also make it possible to test the right operators like |
(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...
|
ef26399
to
16c7959
Compare
35a86f1
to
8d45964
Compare
8d45964
to
c98d2ec
Compare
Sorry I've fallen a bit behind in reviewing. Is this PR ready for review and merge? |
Why was the broadcasting test moved to |
Yep, review ready, and potentially merge ready if you're happy with it.
|
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 |
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 |
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. |
Redundant as we now test broadcasting for each standalone test method
Merging this so that the other PR based on it is easier to review. |
Should resolve #27, if partially. Commits start from #32.
Currently just parametrizes expressions to evaluate. As expected, testing the inplace operators require more thought.