Skip to content

Usability improvements #53

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

Merged
merged 4 commits into from
Dec 23, 2021
Merged

Usability improvements #53

merged 4 commits into from
Dec 23, 2021

Conversation

honno
Copy link
Member

@honno honno commented Dec 22, 2021

This PR firstly introduces the term "xptests", as the brevity should make implementer lives easier (as well as ours)

  • Renames array_api_tests/ to xptests/
  • Renames ARRAY_API_TESTS_MODULE to XPTESTS_MODULE

The README is updated accordingly. Additionally it:

  • Merges in test-coverage.md, as I think keeping everything together is ideal, and knowing what the tests do goes hand-in-hand with the other README content.
  • Modifies the coverage content
    • Readability changes
    • Removes stacking note for now
      • We don't do this explicitly for say elementwise and statistical tests, as we just test that each example has the correct (or correct enough) result, and so this is covered implicitly when covering multiple related examples
      • I think I get what you mean when it comes to linalg and manipulation functions
    • Briefly mention the additional tests
  • Readability changes for the existing README content
  • Remove meta tests comment. I'm personally just writing simple sanity checks when the test logic gets super complicated. Likely these checks don't fail when compared to other faulty test logic problems that we don't even think to catch, so stressing about it seems counter-productive.
  • Minor updates to the testing philosophy stuff

This somewhat covers #46 (comment). A future PR should expand on each function group. I need to mull this over a bit now. It'll definitely cover how "stacking" works in the context of each group.

@honno honno marked this pull request as ready for review December 23, 2021 12:32
@honno honno merged commit 65182a5 into data-apis:master Dec 23, 2021
@honno honno deleted the readme branch December 23, 2021 12:33
@asmeurer
Copy link
Member

asmeurer commented Jan 4, 2022

I don't really like the module rename. The XP abbreviation is pretty opaque to me.

@asmeurer
Copy link
Member

asmeurer commented Jan 4, 2022

Everything else here looks good.

I know the dependencies thing is still a TODO, but a very simple thing we can recommend now is to always start with test_signatures.py, since this will tell you which functions are implemented, and of those which have the expected signature.

We should also mention that the suite is designed so that it can be vendored in a library, if desired. I thought the README already mentioned that, but I guess not. Probably if someone wants to actually do that, though, they should tell us, so that we can start doing actual release tags.

@asmeurer
Copy link
Member

asmeurer commented Jan 4, 2022

Just to be clear, "stacking" in the context of an elementwise function means testing that a function called on an array of values gives the same result as each individual value (an array is a "stack" of its 0-D elements. It might also be called "vectorizing". But it's not actually something I would expect would ever fail for elementwise functions, as opposed to linear algebra where a library might not implement stacking at all.

@rgommers
Copy link
Member

rgommers commented Jan 4, 2022

It'd be good to use "stacking (also called batching)" or some such explanation. Batching is the much more commonly used word.

@honno
Copy link
Member Author

honno commented Jan 4, 2022

I don't really like the module rename. The XP abbreviation is pretty opaque to me.

XP is opaque as an abbreviation, but it's possibly the best option and seems to be the convention the spec runs with. Namely I was interested in making running pytest a lot on specific test cases easier, because even with smart shells the verbosity and underscores of "array_api_tests/" got a bit annoying. Just these last two days I've appreciated the name change a lot. Happy if an end-user was to tie-break on reverting the change heh.

Just to be clear, "stacking" in the context of an elementwise function means testing that a function called on an array of values gives the same result as each individual value (an array is a "stack" of its 0-D elements. It might also be called "vectorizing". But it's not actually something I would expect would ever fail for elementwise functions, as opposed to linear algebra where a library might not implement stacking at all.

This is what confused me, because from my understanding of what you're saying, the elementwise tests don't do this explicitly. Hypothesis will generate related examples such as [[0, 1], [2, 3]]/[2, 3]/2, and so test this implicitly, but for floating-point outputs with rough assertions, we currently (and I'd say appropriately) allow for results not to stack.

@asmeurer
Copy link
Member

asmeurer commented Jan 5, 2022

XP is opaque as an abbreviation, but it's possibly the best option and seems to be the convention the spec runs with.

Where is xp used in the spec? I only see it used once in the "purpose and scope" section as an example variable name for the array namespace. "array_api_tests" on the other hand matches the name of this repo, the array-api repo, and many implementations, and also is pretty self explanatory.

Namely I was interested in making running pytest a lot on specific test cases easier, because even with smart shells the verbosity and underscores of "array_api_tests/" got a bit annoying. Just these last two days I've appreciated the name change a lot. Happy if an end-user was to tie-break on reverting the change heh.

You don't use tab completion? It's the only file in the directory that starts with a. Also unless you are running just a single file you can just use pytest with no argument and it will run the tests, because those are the only ones it will find.

This is what confused me, because from my understanding of what you're saying, the elementwise tests don't do this explicitly. Hypothesis will generate related examples such as [[0, 1], [2, 3]]/[2, 3]/2, and so test this implicitly, but for floating-point outputs with rough assertions, we currently (and I'd say appropriately) allow for results not to stack.

Yes I don't think it's actually implemented yet. That's why I wanted to make a table/checklist for what's missing in the coverage, because I know there's a lot of cases where a test already exists but there's something or other that it doesn't test that it could.

@honno
Copy link
Member Author

honno commented Jan 5, 2022

You don't use tab completion? It's the only file in the directory that starts with a.

I do. It's still annoying as it's faster to write it on remotes, and even locally as my computer is slow. I think it's nice for first-time users generally as well.

Also unless you are running just a single file you can just use pytest with no argument and it will run the tests, because those are the only ones it will find.

Right, but 99% of the time I'm specifying tests. Sure I can stay inside the tests folder, but I prefer working in the root.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants