Utilise RST conversion of spec, generate runtime special cases tests #104
Conversation
This seems fine. If you're worried about name conflicts we could rename it to something more meaningful like |
|
This looks good so far. Some thoughts:
|
| raise ValueParseError(inline_code) | ||
|
|
||
|
|
||
| r_not = re.compile("not (?:equal to )?(.+)") |
There was a problem hiding this comment.
It's good to always use make regular expressions raw strings, even if they don't have backslashes. You don't want to add a backslash escape later only to not realize the string isn't raw.
Just to be sure, this should already be the case with the existing |
| eq = make_eq(i1) | ||
| return eq(i2) | ||
|
|
||
| else: |
There was a problem hiding this comment.
Might be a good idea to code these more defensively like
if value_sign == '+':
...
elif value_sign == '-':
...
else:
raise ValueParseErrorThere was a problem hiding this comment.
Hmm, I believe the regex match should guarantee that value_sign is only "", "-" or "+". A raise here does work well as a sanity check tho.
| if m := r_case.match(line): | ||
| case_str = m.group(1) | ||
| else: | ||
| warn(f"line not machine-readable: '{line}'") |
There was a problem hiding this comment.
We really want this to be an error, not a warning, I think.
There was a problem hiding this comment.
And even if we didn't want this to be an error, I would avoid using warnings. pytest is just going to list warnings at the end, but modules like NumPy already emit dozens of warnings when running the tests, which can be ignored, and any legitimate warning would be completely lost.
There was a problem hiding this comment.
I'd rather not raise an error here as it's not critical (unlike the assertions I make in stubs.py), so would unnecessarily halt use of the test suite for the myriad of reasons a user could have an out-of-sync array-api submodule.
I agree warnings are pretty awkward in practice, but I still think they're the only appropriate choice we have. I was actually using this in setup.cfg when developing all the parsing code paths:
[tool:pytest]
filterwarnings =
ignore::ImportWarning
ignore::RuntimeWarning
ignore::DeprecationWarning
Actually, I just realized the (git) submodule breaks the ability to vendor array_api_tests. I think we can make vendoring possible by allowing copying Either way, it makes sense to keep |
Yep, I was initially using branch resets like you did, but ended up with a solution that doesn't need it.
I did not know this was a thing, very nifty!
Ahh
Hmm, I've seen uppercase elsewhere, and
I'll have a quick look at replacing the submodule, but as I'm not tackling module stubbing/signature tests in this PR I won't prioritise this just yet.
I do have a few hard, non-test assertions for all of this, but definitely something I can review.
Hmm, as I'm sure you'd agree it'd be ideal if we didn't hardcode things from a maintenance perspective. I guess the question is if it's too annoying to ask users to pull and deal with an
Yep, once I stabilise things 😅
Yep I believe |
I don't think we should require vendored users to use a submodule. If we don't commit the signature files, we need a way for users to do it, like a script that packages a standalone vendor module that people can use, meaning they have to run that each time they update. So it's a question of trading some maintenance upside for us with some usability for end-users. OTOH maybe this is just a reason why we should upload a package so that people who vendor can copy the from that. |
Ok yeah I see. What do we mean when we say vendored though? Like the only practical use we're seeing right now is pulling the repo and running it in CI, where the submodule requirement is only a minor inconvenience. If a different use case arises then it may be appropriate to support it only then. Anywho, I think if we hard copy |
|
#40 might be pretty tricky. It seems there's a fair few existing special case tests that don't generate that many relevant examples already, so it could be apt to instead generate a single minimal reproducer per special case for now, as generating relevant Hypothesis arrays is tricky enough for a PR in its own right. Will explore this more later anywho. Special cases in this PR that fail Hypothesis' health check for unmodified array strategiestest_unary[abs-x_i == -0 -> +0] test_unary[acos-x_i == 1 -> +0] test_unary[acosh-x_i == 1 -> +0] test_unary[acosh-x_i == +infinity -> +infinity] test_unary[asin-x_i == +0 -> +0] test_unary[asin-x_i == -0 -> -0] test_unary[asinh-x_i == -0 -> -0] test_unary[asinh-x_i == -infinity -> -infinity] test_unary[atan-x_i == +0 -> +0] test_unary[atan-x_i == -0 -> -0] test_unary[atanh-x_i == -1 -> -infinity] test_unary[atanh-x_i == +1 -> +infinity] test_unary[atanh-x_i == -0 -> -0] test_unary[ceil-x_i == -0 -> -0] test_unary[cos-x_i == -0 -> 1] test_unary[cos-x_i == +infinity -> NaN] test_unary[cosh-x_i == -0 -> 1] test_unary[exp-x_i == -0 -> 1] test_unary[exp-x_i == -infinity -> +0] test_unary[expm1-x_i == -0 -> -0] test_unary[expm1-x_i == +infinity -> +infinity] test_unary[expm1-x_i == -infinity -> -1] test_unary[floor-x_i == +infinity -> +infinity] test_unary[floor-x_i == -infinity -> -infinity] test_unary[floor-x_i == -0 -> -0] test_unary[log-x_i == 1 -> +0] test_unary[log1p-x_i == -1 -> -infinity] test_unary[log1p-x_i == -0 -> -0] test_unary[log1p-x_i == +0 -> +0] test_unary[log2-x_i == 1 -> +0] test_unary[log10-x_i == 1 -> +0] test_unary[round-x_i == +infinity -> +infinity] test_unary[round-x_i == -0 -> -0] test_unary[sin-x_i == -0 -> -0] test_unary[sinh-x_i == -0 -> -0] test_unary[sinh-x_i == +infinity -> +infinity] test_unary[sinh-x_i == -infinity -> -infinity] test_unary[sqrt-x_i == -0 -> -0] test_unary[sqrt-x_i == +infinity -> +infinity] test_unary[tan-x_i == +0 -> +0] test_unary[tan-x_i == -0 -> -0] test_unary[tanh-x_i == +0 -> +0] test_unary[tanh-x_i == -0 -> -0] test_unary[tanh-x_i == +infinity -> +1] test_unary[trunc-x_i == -0 -> -0] test_binary[add-x1_i == +infinity and x2_i == -infinity -> NaN] test_binary[add-x1_i == -infinity and x2_i == +infinity -> NaN] test_binary[add-x1_i == +infinity and x2_i == +infinity -> +infinity] test_binary[add-x1_i == -infinity and x2_i == -infinity -> -infinity] test_binary[add-x1_i == +infinity and isfinite(x2_i) -> +infinity] test_binary[add-x1_i == -infinity and isfinite(x2_i) -> -infinity] test_binary[add-isfinite(x1_i) and x2_i == -infinity -> -infinity] test_binary[add-x1_i == -0 and x2_i == -0 -> -0] test_binary[add-x1_i == -0 and x2_i == +0 -> +0] test_binary[add-x1_i == +0 and x2_i == -0 -> +0] test_binary[add-x1_i == +0 or x1_i == -0 and isfinite(x2_i) and x2_i != 0 -> x2_i] test_binary[add-isfinite(x1_i) and x1_i != 0 and x2_i == +0 or x2_i == -0 -> x1_i] test_binary[add-isfinite(x1_i) and x1_i != 0 and x2_i == -x1_i -> +0] test_binary[atan2-x1_i > 0 and x2_i == +0 -> roughly +pi/2] test_binary[atan2-x1_i > 0 and x2_i == -0 -> roughly +pi/2] test_binary[atan2-x1_i == +0 and x2_i > 0 -> +0] test_binary[atan2-x1_i == +0 and x2_i == -0 -> roughly +pi] test_binary[atan2-x1_i == +0 and x2_i < 0 -> roughly +pi] test_binary[atan2-x1_i == -0 and x2_i > 0 -> -0] test_binary[atan2-x1_i == -0 and x2_i == +0 -> -0] test_binary[atan2-x1_i == -0 and x2_i == -0 -> roughly -pi] test_binary[atan2-x1_i == -0 and x2_i < 0 -> roughly -pi] test_binary[atan2-x1_i < 0 and x2_i == +0 -> roughly -pi/2] test_binary[atan2-x1_i < 0 and x2_i == -0 -> roughly -pi/2] test_binary[atan2-x1_i > 0 and isfinite(x1_i) and x2_i == +infinity -> +0] test_binary[atan2-x1_i > 0 and isfinite(x1_i) and x2_i == -infinity -> roughly +pi] test_binary[atan2-x1_i < 0 and isfinite(x1_i) and x2_i == +infinity -> -0] test_binary[atan2-x1_i < 0 and isfinite(x1_i) and x2_i == -infinity -> roughly -pi] test_binary[atan2-x1_i == -infinity and isfinite(x2_i) -> roughly -pi/2] test_binary[atan2-x1_i == +infinity and x2_i == +infinity -> roughly +pi/4] test_binary[atan2-x1_i == +infinity and x2_i == -infinity -> roughly +3pi/4] test_binary[atan2-x1_i == -infinity and x2_i == +infinity -> roughly -pi/4] test_binary[atan2-x1_i == -infinity and x2_i == -infinity -> roughly -3pi/4] test_binary[divide-x1_i == +infinity or x1_i == -infinity and x2_i == +infinity or x2_i == -infinity -> NaN] test_binary[divide-x1_i == +0 and x2_i > 0 -> +0] test_binary[divide-x1_i == -0 and x2_i > 0 -> -0] test_binary[divide-x1_i == +0 and x2_i < 0 -> -0] test_binary[divide-x1_i == -0 and x2_i < 0 -> +0] test_binary[divide-x1_i > 0 and x2_i == +0 -> +infinity] test_binary[divide-x1_i > 0 and x2_i == -0 -> -infinity] test_binary[divide-x1_i < 0 and x2_i == +0 -> -infinity] test_binary[divide-x1_i < 0 and x2_i == -0 -> +infinity] test_binary[divide-x1_i == +infinity and isfinite(x2_i) and x2_i > 0 -> +infinity] test_binary[divide-x1_i == -infinity and isfinite(x2_i) and x2_i > 0 -> -infinity] test_binary[divide-isfinite(x1_i) and x1_i > 0 and x2_i == +infinity -> +0] test_binary[divide-isfinite(x1_i) and x1_i > 0 and x2_i == -infinity -> -0] test_binary[divide-isfinite(x1_i) and x1_i < 0 and x2_i == +infinity -> -0] test_binary[floor_divide-x1_i == +infinity or x1_i == -infinity and x2_i == +infinity or x2_i == -infinity -> NaN] test_binary[floor_divide-x1_i == +0 and x2_i > 0 -> +0] test_binary[floor_divide-x1_i == -0 and x2_i > 0 -> -0] test_binary[floor_divide-x1_i == +0 and x2_i < 0 -> -0] test_binary[floor_divide-x1_i == -0 and x2_i < 0 -> +0] test_binary[floor_divide-x1_i > 0 and x2_i == +0 -> +infinity] test_binary[floor_divide-x1_i > 0 and x2_i == -0 -> -infinity] test_binary[floor_divide-x1_i < 0 and x2_i == +0 -> -infinity] test_binary[floor_divide-x1_i < 0 and x2_i == -0 -> +infinity] test_binary[floor_divide-x1_i == +infinity and isfinite(x2_i) and x2_i > 0 -> +infinity] test_binary[floor_divide-x1_i == +infinity and isfinite(x2_i) and x2_i < 0 -> -infinity] test_binary[floor_divide-x1_i == -infinity and isfinite(x2_i) and x2_i > 0 -> -infinity] test_binary[floor_divide-x1_i == -infinity and isfinite(x2_i) and x2_i < 0 -> +infinity] test_binary[floor_divide-isfinite(x1_i) and x1_i > 0 and x2_i == +infinity -> +0] test_binary[floor_divide-isfinite(x1_i) and x1_i > 0 and x2_i == -infinity -> -0] test_binary[floor_divide-isfinite(x1_i) and x1_i < 0 and x2_i == +infinity -> -0] test_binary[floor_divide-isfinite(x1_i) and x1_i < 0 and x2_i == -infinity -> +0] test_binary[multiply-x1_i == +infinity or x1_i == -infinity and x2_i == +0 or x2_i == -0 -> NaN] test_binary[multiply-x1_i == +0 or x1_i == -0 and x2_i == +infinity or x2_i == -infinity -> NaN] test_binary[pow-x2_i == -0 -> 1] test_binary[pow-x1_i == NaN and not (x2_i == 0) -> NaN] test_binary[pow-abs(x1_i) > 1 and x2_i == -infinity -> +0] test_binary[pow-abs(x1_i) == 1 and x2_i == +infinity -> 1] test_binary[pow-abs(x1_i) == 1 and x2_i == -infinity -> 1] test_binary[pow-x1_i == 1 and not (x2_i == NaN) -> 1] test_binary[pow-abs(x1_i) < 1 and x2_i == -infinity -> +infinity] test_binary[pow-x1_i == +infinity and x2_i > 0 -> +infinity] test_binary[pow-x1_i == +infinity and x2_i < 0 -> +0] test_binary[pow-x1_i == -infinity and x2_i > 0 and x2_i.is_integer() and x2_i % 2 == 1 -> -infinity] test_binary[pow-x1_i == -infinity and x2_i > 0 and not (x2_i.is_integer() and x2_i % 2 == 1) -> +infinity] test_binary[pow-x1_i == -infinity and x2_i < 0 and x2_i.is_integer() and x2_i % 2 == 1 -> -0] test_binary[pow-x1_i == +0 and x2_i > 0 -> +0] test_binary[pow-x1_i == +0 and x2_i < 0 -> +infinity] test_binary[pow-x1_i == -0 and x2_i > 0 and x2_i.is_integer() and x2_i % 2 == 1 -> -0] test_binary[pow-x1_i == -0 and x2_i > 0 and not (x2_i.is_integer() and x2_i % 2 == 1) -> +0] test_binary[pow-x1_i == -0 and x2_i < 0 and x2_i.is_integer() and x2_i % 2 == 1 -> -infinity] test_binary[pow-x1_i == -0 and x2_i < 0 and not (x2_i.is_integer() and x2_i % 2 == 1) -> +infinity] 07/03 update: Generating good examples for unary functions—and binary functions w/ one "sub" case per I discussed this particular issue up in the internal Data APIs meeting, as finding a solution has been a bit frustrating. Basically there was three approaches I envisaged:
I have a working solution using Hypothesis, but I actually think it's use of Hypothesis is a bit inside-baseball and so not ideal for the long-term maintainability of the suite. I was thinking now that I could half-compromise with hard coding the combinations e.g. In any case, I'll just get a solution in soon, and probably avoid revisiting this niche problem for a long time heh. |
|
Vendoring meaning copying the test suite into the library's own test suite. It's true no one has been doing that yet, but it is something that we've supported up until now. |
|
I resolved #40! I've documented the relevant areas, although generally there's a few small spots I'd like to cover still. Generally I'm happy for this to get merged as-is. Here's the fairly simple TODOs I can think of for now or future PRs:
And to fully replace
Also, I'm not quite sold it's worth supporting hypothetical situations where users might not be able to use git submodules, which might be the case if the test suite was to be vendored. But in any case, I'd really like to not do this until I write a pre-commit script, lest we get out-of-sync stubs like last time.
|
|
I might write a separate issue on vendoring, just to note in today's meeting we discussed this... I realise a git submodule which is already inside a folder is not certainly not ideal heh. The current options seem to be:
The timeline where libraries actually vendor might be a while though, which means we could give this some thought (and write an issue). Generally, it seems to be a good idea to
|
honno
force-pushed
the
rst-spec
branch
2 times, most recently
from
3a2900d
to
02dfb64
Compare
Also add some more granular documentation to it
|
@asmeurer happy to merge on my end—not critical but would nice to get this in soon so we can get feedback from say Dask about these tests. See #104 (comment) for notes on further work and vendoring. |
Ok, this ones a biggie. It's not finished yet, but thought you'd like to take a look @asmeurer.
The first big thing is making use of the RST spec (data-apis/array-api#343). This is achieved by adding the spec repo
array-apias a git submodule inside of ourarray-api-tests/folder, adding the signatures package to the Python path, and extracting all the function objects (or class in the case of the array object).(I tried not adding the signatures package to the Python path, as it's rather unseemly and potentially problematic to users, but I struggled trying to parse the functions/methods with the nifty
importlibutils due to relative imports.)Next we have the special case tests.
float) of the respect array element. These are constructed inparse_cond(). Results are now checked against the scalar too, with a result-checker function constructed inparse_result().TWO_ARGS_EQUAL,TWO_ARGS_EQUAL__LESS, etc. 🎉parse_cond()/parse_result()/something else needs updating to cover any new special cases with unforeseen properties.assume()to the generated examples, so Hypothesis will complain if we're not actually generating relevant examples, resolving Make special case tests only use relevant arrays #40I still have a fair bit to do, but I'm pretty happy with the architecture now. I will probably touch the signature tests or module stubbing in a later PR.
Note right now there's a bug with the conditions giving false-negatives/postives, due to some pass-by-reference problems when I defined local functions in loops 😅