hypothesis
hypothesis copied to clipboard
`extra.array_api` versioning + complex support
WIP PR to resolve https://github.com/HypothesisWorks/hypothesis/issues/3422. That means 1) complex support 2) versioning namespaces. Not particularly looking for a thorough review just yet, but would of-course appreciate feedback and ideas.
Prior brain dump
Complex support
Per point 1, supporting complex arrays has consisted of my initial proposal
- Generates complex dtype objects in
xps.numeric_dtypes()
, as per how "numeric" is treated in the draft spec.- Introduce
xps.real_dtypes()
to generate integer and floating-point dtype objects, as per how "real" is treated in the draft spec.- Introduce
xps.complex_dtypes()
to generate complex dtypes.- Support generating complex values in
xps.from_dtype()
.
Versioned strategy namespaces
Per point 2, make_strategies_namespace()
now takes an api_version
kw-only argument per @Zac-HD's suggestion. The argument
- defaults to
...
, which indeed tries known versions (descending) and returns the first that works. - can be a released versions string (i.e. just
"2021.12"
for now), which forcibly returns a strategies namespace for the given version. - can be
"draft"
, which lets us access a namespace with no guarantees (documented as such). The expectation is that the namespace would follow the draft spec. This is useful to implement and internally test features before spec releases (e.g. complex numbers)... and helps me out witharray-api-tests
:sweat_smile: - Anything else, including
None
, raises a hopefully useful error. Might need to mull it over, and write additional context if we detectNone
.
Currently in a 2021.12
namespace, numeric_dtypes()
doesn't generate complex numbers, and complex_dtypes()
doesn't exist. draft
conversely has the complex support as outlined a bit above.
Internal test suite
The big TODO is updating the test suite. As of writing, currently I've introduced xp-agnostic tests for make_strategies_namespace()
, and tests for new complex behaviour.
Parametrizing versioned namespaces?
Per tests/array_api/conftest.py
(see #3085 and #3275), we need to consider how versioning interacts with parametrization of array modules. See its README.
I think ideally
- when specifying an array module, default to just testing with the strategies namespace we get from
make_strategies_namespace(...)
. - add support for specifying a specific version for said array module(s).
- by default, test our
mock_xp
on"draft"
. Currently we test with NumPy-proper if available, but this is limiting in testing newer features (e.g. complex-numbers). Or maybe test bothmock_xp
on"draft"
andnumpy
on inferred latest version.
Version-specific testing
Mark tests which are only for YYYY.MM+ versions?
For the complex-related tests which get parametrize via contest.py
, my idea was/is to introduce a marker xp_min_version(api_version)
, e.g.
# test_scalar_dtypes.py
@pytest.mark.xp_min_version("draft")
def test_can_generate_complex_dtypes(xp, xps):
...
# test_arrays.py
dtype_name_params = ["bool",] + list(REAL_NAMES) + [
pytest.param(n, marks=pytest.mark.xp_min_draft("draft")]
]
@pytest.mark.parametrize("dtype_name", dtype_name_params)
def test_draw_arrays_from_dtype(...):
...
@pytest.mark.parametrize("dtype_name", dtype_name_params)
def test_draw_arrays_from_scalar_names(...):
...
I thought I could get the xps
argument from a pytest item in the collection_modifyitems
hook, which would hold a newly introduced api_version
attribute. For items with xp_min_version()
markers, that xps.api_version
value could of been checked against the min version (e.g. from <marker>.args[0]
), where when "less-than" a skip
marker would be added to the item... my first go didn't work out heh, so will sleep on it.
For some tests, change test behaviour on runtime?
Also additionally xps.api_version
could maybe change behaviour of tests like
# test_scalar_dtypes.py
def test_can_generate_numeric_dtypes(xp, xps):
if api_version_gt(xps.api_version, "2021.12"):
dtype_names = <includes complex dtypes>
else:
dtype_names = <doesn't include complex dtypes>
numeric_dtypes = [getattr(xp, n) for n in dtype_names]
assert_all_examples(xps.numeric_dtypes(), lambda dtype: dtype in numeric_dtypes)
Might want to create separate tests, will mull over.
TODO:
- [ ] Review the diff and see if there's anything I could clean up easily.
- [ ]
RELEASE.rst
and other things CI picks on - [x] Update the test suite.
- [ ] Update/add some API docs.
- [ ] Add test docstrings (namely to force me to review them heh).
Thanks for the suggestions! Other than an updated TODO list, think I'm now happy with the API and general architecture things, but something to sleep on.
I updated the tests/array_api/
README which should explain my current solution to namespace version paramtrization. Basically by default the latest supported version is used, and a new env lets you specify a specific version. I didn't introduce an "all" option as ugh I need to think about it :upside_down_face: I guess a future problem is you can end up not testing with all released versions by default, just for now on CI you should have draft covered (by the mock) and 2021.12 covered by numpy.array_api
.
Having gone through it again, I don't think there are any major changes; just a little polish and I'll be happy to merge.
I updated the tests/array_api/ README which should explain my current solution to namespace version paramtrization. Basically by default the latest supported version is used, and a new env lets you specify a specific version. I didn't introduce an "all" option as ugh I need to think about it 🙃 I guess a future problem is you can end up not testing with all released versions by default, just for now on CI you should have draft covered (by the mock) and 2021.12 covered by numpy.array_api.
I'm fine with leaving a note (see comment above), and otherwise deciding that we'll deal with this later. More, smaller PRs is good!
Linting, type checking, and release notes - then I think we're ready to merge!
(though FYI I'm away thurs-mon, so might get to this late next week)
So in regards to xp.__array_api_version__
https://github.com/HypothesisWorks/hypothesis/pull/3456#discussion_r976412249, the PR now uses that for inferrence, and ditches the funky x.__array_namespace__()
method (save here in case). Tests have been updated accordingly.
For the test/array_api/
conf, I now just default to testing with the mock, as it seems rather bothersome to raise a warning or whatever for default runs of the test suite (for CI but also developer experience). README.md
updated. Also numpy.array_api
might not exist/be updated soon enough? https://github.com/numpy/numpy/issues/22252
Windows tests seem to be failing because the custom markers registered in tests/array_api/conftest.py
are well not being registered heh. Will explore next week.
Thanks for the great review comments, should be all addressed now.
I think after this round we're probably just waiting for a decision on complex-dtype introspection?
Technically this PR is compliant to the current draft spec, so it seems waiting is a question of do we want an inevitable future PR.
I'm 99% sure complex support for xp.finfo()
(https://github.com/data-apis/array-api/pull/484) will get merged, given we agreed upon it in a recent consortium meeting. Spec triage is always variable and our resident reviewer is currently out, so it might not get merged say this week... we could shoot the gun here even?
Also I noticed nps.from_dtype()
currently doesn't support min/max magnitude arguments—once we resolve complex-dtype introspection, it's okay to support those args for xps.from_dtype()
at least I assume?
I noticed
nps.from_dtype()
currently doesn't support min/max magnitude arguments—once we resolve complex-dtype introspection, it's okay to support those args forxps.from_dtype()
at least I assume?
Absolutely, also happy to support in xps
too when that makes sense - and since it's a logically separate feature, please post it as a separate PR!
🚢🚢:shipit:
Awesome, thanks again Zac!