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

Robuster deeply nested structures #8730

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

fjetter
Copy link
Member

@fjetter fjetter commented Jun 24, 2024

Closes #8700

The reason why I said that those tests are not great is because the thresholds were arbitrary and therefore dependent on CPython, pytest and dask code.

This changes the way the deeply nested structure is generated in a way that it generates a structure that is multiple times that of the recursion depth so every attempt to dive into this will inevitably fail with a recursion error.

It works... but python3.12 is horribly slow. I haven't profiled this, yet. On py3.10 this runs in about 0.1s while on py3.12 this runs for 20-30s. This worries me for two reasons

  1. I don't really want to have this test take up this much time. It's not that valuable
  2. More importantly, if this runs for so long, I'm worried that the profile thread also occasionally just gets stuck in this deep loop and eats up all the CPU time, congests the GIL, etc.

I haven't run any profiles yet (mostly because pyspy doesn't work on python3.12) but I welcome anybody else to peak into this

Copy link
Contributor

Unit Test Results

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

    29 files  ±    0      29 suites  ±0   11h 10m 1s ⏱️ - 19m 46s
 4 065 tests +    1   3 957 ✅  -     3     97 💤 ± 0  10 ❌ +3  1 🔥 +1 
52 489 runs   - 3 492  50 359 ✅  - 3 449  2 111 💤  - 52  17 ❌ +7  2 🔥 +2 

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

Results for commit 3c06283. ± Comparison against base commit 33a281f.

@fjetter
Copy link
Member Author

fjetter commented Jun 24, 2024

yeah, py3.12 runs into a timeout (and the others raise a genuine recursion error instead of a type error, i.e. the recursion is now hit before we enter pickle/msgpack)

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.

test_deeply_nested_structures Failed: DID NOT RAISE <class 'TypeError'> with python3.12
1 participant