[red-knot] Stabilize negation_reverses_subtype_order property test
#16764
Conversation
There was a problem hiding this comment.
Thanks, this looks great! It looks like this change means that the negation_reverses_subtype_order property test now passes consistently; could you move it from the flaky submodule in property_tests.rs to the stable submodule?
I verified that it now passes consistently by running QUICKCHECK_TESTS=2000000 cargo test --release -p red_knot_python_semantic -- --ignored types::property_tests::flaky::negation_reverses_subtype_order, which quickly fails on main but not on this PR branch.
|
This comment was marked as off-topic.
This comment was marked as off-topic.
For this change on its own, the benchmarks are in fact entirely neutral. I think the slowdown observed on that earlier PR must come from somewhere else. Thanks for the PR, this is great! |
negation_reverses_subtype_order property test
|
I'm so sorry -- I closed this meaning to close and reopen it, but now GitHub isn't letting me reopen it. I'll try to open a fresh PR from this branch. |
|
#16801. Sorry again; don't know why this happened :-( |
…16801) ## Summary This is a re-creation of #16764 by @mtshiba, which I closed meaning to immediately reopen (GitHub wasn't updating the PR with the latest pushed changes), and which GitHub will not allow me to reopen for some reason. Pasting the summary from that PR below: > From #16641 > > As stated in this comment (#16641 (comment)), the current ordering implementation for intersection types is incorrect. So, I will introduce lexicographic ordering for intersection types. ## Test Plan One property test stabilised (tested locally with `QUICKCHECK_TESTS=2000000 cargo test --release -p red_knot_python_semantic -- --ignored types::property_tests::stable::negation_reverses_subtype_order`), and existing mdtests that previously failed now pass. Primarily-authored-by: [mtshiba](https://github.com/astral-sh/ruff/commits?author=mtshiba) --------- Co-authored-by: Shunsuke Shibayama <sbym1346@gmail.com>
Summary
From #16641
As stated in this comment (#16641 (comment)), the current ordering implementation for intersection types is incorrect. So, I will introduce lexicographic ordering for intersection types.
Test Plan
As can be seen here, it appears that the execution time of
red_knot_check_filewill slow down by about 3% when this change is merged. The slow-down itself is unavoidable given that we have been skipping processes that should have been done, but please let me know if there is a better ordering algorithm or some sort of salsa query technique.