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

Remove expensive tokenization for key uniqueness check #9009

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

phofl
Copy link
Collaborator

@phofl phofl commented Feb 13, 2025

Closes #xxxx

  • Tests added / passed
  • Passes pre-commit run --all-files

This can be pretty expensive and the use of this check is more or less nonexistent. Comparing the dependencies itself is sufficient for what we want to do here.

@phofl phofl requested a review from fjetter as a code owner February 13, 2025 10:43
Copy link
Member

@fjetter fjetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also known for many false positives in the xarray space. We hope that the root cause that led us to implement this check will be fixed medium term anyhow by our push for array expressions

Copy link
Contributor

github-actions bot commented Feb 13, 2025

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

    27 files  ±  0      27 suites  ±0   11h 24m 18s ⏱️ + 1m 14s
 4 105 tests  -  15   3 987 ✅  -  19    111 💤 ±0  6 ❌ +3  1 🔥 +1 
51 473 runs   - 194  49 169 ✅  - 199  2 297 💤 +1  6 ❌ +3  1 🔥 +1 

For more details on these failures and errors, see this check.

Results for commit d4a8c36. ± Comparison against base commit 33b229a.

This pull request removes 15 tests.
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_after_previous_is_done[False-less]
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_after_previous_is_done[False-more]
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_after_previous_is_done[False-same]
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_after_previous_is_done[True-less]
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_after_previous_is_done[True-more]
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_after_previous_is_done[True-same]
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_before_previous_is_done[less]
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_before_previous_is_done[more]
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_before_previous_is_done[same]
distributed.tests.test_scheduler ‑ test_resubmit_different_task_same_key_many_clients
…

♻️ This comment has been updated with latest results.

@phofl phofl merged commit d156473 into dask:main Feb 13, 2025
25 of 31 checks passed
@phofl phofl deleted the tokenize-check branch February 13, 2025 14:25
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