Repository navigation
TST: cover reflected scalar operators - #471
Conversation
| ("__ror__", "bitwise_or", operator.or_, dh.bool_and_all_int_dtypes, 6, (1, 2)), | ||
| ("__rxor__", "bitwise_xor", operator.xor, dh.bool_and_all_int_dtypes, 6, (1, 2)), | ||
| ("__rlshift__", "bitwise_left_shift", operator.lshift, dh.all_int_dtypes, 3, (1, 2)), | ||
| ("__rrshift__", "bitwise_right_shift", operator.rshift, dh.all_int_dtypes, 8, (1, 2)), |
There was a problem hiding this comment.
For consistency with the rest of the test suite, it'd be better to use hypothesis-generated values.
There was a problem hiding this comment.
The tests now draw scalars, array elements, and shapes with Hypothesis; the focused cases pass locally with array-api-strict and NumPy.
ev-br
left a comment
ev-br
left a comment
There was a problem hiding this comment.
Looks reasonable, will test some more locally. Meanwhile, could you please add the try/except "repro_snippet" handling , see elsewhere in this test module for examples.
71b70e1 to
e0da98d
Compare
ev-br
left a comment
ev-br
left a comment
There was a problem hiding this comment.
As noted below, the test is a written in a bit unorthodox way, and the usual way of controlling the coverage of a test run does not quite work with this test. I'm going to add a comment to this effect and see if the difference becomes a problem with our regular usage. In which case, the test will need to be refactored to use hypothesis strategies for dtypes, too.
Other potential follow-up are
- test
__rop__s with both scalars and arrays: it currently only testsscalar <op> arraycombinations. - test consistency of
__op__(a, b)and__rop__(b, a).
Adding all of these is quite a project, so I think we can go land the PR as is, to provide at least some smoke testing of reflected dunder ops.
Working on any of these follow-ups would be most welcome @Boulea7 if you're interested.
I tested it locally with numpy, torch, cupy, dask.array and jax.numpy, and it all seems to work.
Am going to land it now, thank you @Boulea7 !
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "method, func_name, op, dtype", list(make_reflected_scalar_params()) |
There was a problem hiding this comment.
parametrizing over the full lists of dtypes from dtype_helpers is a bit unorthodox: the rest of the test suite uses the matching strategies from hypothesis_helpers. The difference seems minor, but where it bites is that with the rest of the test suite we typically use --max-examples= to control the coverage [1]. Using a large value of examples makes it instead draw many values and still loop over all dtypes. Which does not really increase the coverage and just makes it slower.
[1] so that a typical test incantation for a thorough test is
$ ARRAY_API_TESTS_SKIP_DTYPES=uint16,uint32,uint64 ARRAY_API_TESTS_MODULE=array_api_compat.cupy pytest array_api_tests/test_operators_and_elementwise_functions.py -kreflected -v --max-examples=10_000
The operator tests exercise array-left expressions but do not check scalar-left reflected arithmetic and bitwise operators. This adds Hypothesis-generated cases for the seven reflected arithmetic methods and five reflected bitwise methods, checking dtype, shape, and values against the corresponding Array API functions. Inputs stay small and finite, with nonzero divisors, small whole-number exponents, and valid shift counts.
The parametrized cases are materialized as a list so collection does not rely on pytest's deprecated support for generator arguments.
Validation: 115 reflected-operator cases pass with array-api-strict 2.6.1 (API 2025.12) and pytest 9.1.1, with
PytestRemovedIn10Warningtreated as an error. The full suite was not run locally.Refs #460