Adjust merge and categorical tests for dask-expr#10762
Conversation
There was a problem hiding this comment.
One question and nit about pytest.skip uses is all. LGTM otherwise, thank you!
Edit: The 3.12 failures ought to be fixed after update, fixed in dask/dask-expr#689. The other seems unrelated...
FAILED dask/tests/test_delayed.py::test_name_consistent_across_instances - AssertionError: assert 'identity-096...f6f4120187a73' == 'identity-4f3...7c3ac07f7201a'
- identity-4f318f3c27b869239e97c3ac07f7201a
+ identity-09699241ef8c5469dd9f6f4120187a73
| errors = ( | ||
| (ValueError, TypeError) | ||
| if pa is None | ||
| else (ValueError, TypeError, ArrowNotImplementedError) | ||
| ) |
There was a problem hiding this comment.
I'm guessing the additional error also depends on DASK_EXPR_ENABLED, is that right? Is it worth making this conditional as well? (My guess is no. :) )
There was a problem hiding this comment.
Probably not no, the tests was already ugly before this pr :)
| def test_merge_asof_indexed(): | ||
| if DASK_EXPR_ENABLED: | ||
| pytest.skip("merge_asof not available yet in dask-expr") |
There was a problem hiding this comment.
Based on pytest docs, I think xfail is more appropriate in these situations.
| def test_merge_asof_indexed(): | |
| if DASK_EXPR_ENABLED: | |
| pytest.skip("merge_asof not available yet in dask-expr") | |
| @pytest.mark.xfail(DASK_EXPR_ENABLED, reason="merge_asof not available ye tin dask-expr") | |
| def test_merge_asof_indexed(): |
There was a problem hiding this comment.
That doesn't work unfortunately, we can't evaluate the config in these skip contexts, don't know why
There was a problem hiding this comment.
hmm, is that just because it's the decorator.. would replacing the in-function skip with xfail also not work? Just curious if there is a difference in the two or if when used as a decorator.
There was a problem hiding this comment.
It's the decorator, inline all of them work
2 participants
pre-commit run --all-files