Skip to content
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

Avoid calling hh.arrays with dtype=None for complex types #313

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cbourjau
Copy link
Contributor

Issue description

ndonnx does not (yet) support complex data types. We signal that by setting ARRAY_API_TESTS_SKIP_DTYPES="complex64,complex128". During the collection phase hypothesis_helpers.complex_dtypes is set to None. However, downstream calls to hypothesis_helpers.arrays appear to not be able to handle dtype=None as an argument, ultimately leading to an error when looking up the data type such as:

________________ ERROR collecting array_api_tests/test_operators_and_elementwise_functions.py ________________________
api-coverage-tests/array_api_tests/test_operators_and_elementwise_functions.py:1066: in <module>
    @given(hh.arrays(dtype=hh.complex_dtypes, shape=hh.shapes()))
api-coverage-tests/array_api_tests/hypothesis_helpers.py:73: in arrays
    elements = from_dtype(dtype)
api-coverage-tests/array_api_tests/hypothesis_helpers.py:44: in from_dtype
    min_, max_ = dh.dtype_ranges[component_dtype]
api-coverage-tests/array_api_tests/dtype_helpers.py:85: in __getitem__
    raise KeyError(f"{key!r} not found")
E   KeyError: 'None not found'

Since data types are looked up using __eq__ things appear to work if any of the data types compare equal to None which is the case for numpy.dtype("float64"), for example.

Solution

This PR simply guards each affected tests (i.e. tests that work on hypothesis_helpers.complex_dtypes) with a respective if statement. This works just fine in practice for us, but I'm sure there is a better fix out there. I tried to set complex_dtypes = (), which seems reasonable, but that lead to other errors.

@asmeurer
Copy link
Member

Does using pytest.skip not work here? That would be preferred if it can work.

@asmeurer
Copy link
Member

You could also try modifying hh.arrays so that it fails at example draw time instead of at function call time when dtypes=().

I'm curious how this is able to work when complex dtypes aren't enabled due to older standard versions. Maybe that isn't ever actually tested anywhere, since even array-api-strict's older standard flags doesn't actually support versions prior to adding complex support.

We definitely do want to support skipping complex dtypes with ARRAY_API_TESTS_SKIP_DTYPES, but that hasn't been tested before.

@cbourjau
Copy link
Contributor Author

cbourjau commented Dec 1, 2024

Apologies for the late reply!

Does using pytest.skip not work here? That would be preferred if it can work.

I'm not sure how we may use pytest.skip here. The error occurs already during pytest's collection phase meaning that neither pytest.mark.skipif nor pytest.skip are applicable, AFAICT. Calling the latter from hypothesis_helpers.arrays yields:

============================================== ERRORS ==============================================
___________________________ ERROR collecting array_api_tests/test_fft.py ___________________________
Using pytest.skip outside of a test will skip the entire module. If that's your intention, pass `allow_module_level=True`. If you want to skip a specific test or an entire class, use the @pytest.mark.skip or @pytest.mark.skipif decorators.

You could also try modifying hh.arrays so that it fails at example draw time instead of at function call time when dtypes=().

Might you have an example where something like that is already done in this project? I'm rather inexperienced with hypothesis.

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.

2 participants