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

Python 3.13 fixes #11185

Merged
merged 2 commits into from
Jun 18, 2024
Merged

Python 3.13 fixes #11185

merged 2 commits into from
Jun 18, 2024

Conversation

AdamWill
Copy link
Contributor

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

This fixes two failures when running the test suite on Python 3.13.

  • dataclasses.replace() changed the type of the exception it raises in a couple of cases, one of which causes us to also raise a different exception type with a misleading message. We could adopt the same exception type change and just make sure we use the correct message, I guess, but that seems like an interface change, this keeps the behaviour the same as it was before.
  • As per the what's new page, "Compiler now strip indents from docstrings." This broke the expectation of test_derived_from. We can fix it just by not expecting the indentation, I don't think we really need to enforce that it's there for older Python versions, we're just checking that our extra docstring text got in there, not policing indentation.

In Python 3.13, `dataclasses.replace()` now raises a `TypeError`
rather than a `ValueError` when a field is declared with
init=False (or when an init variable is not specified). We were
relying on it raising a `ValueError` in this case. This fix
should keep dask's interface consistent across Python versions.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
In Python 3.13, it is not any more:
https://docs.python.org/3.13/whatsnew/3.13.html#other-language-changes
"Compiler now strip indents from docstrings."

Signed-off-by: Adam Williamson <awilliam@redhat.com>
@GPUtester
Copy link
Collaborator

Can one of the admins verify this patch?

Admins can comment ok to test to allow this one PR to run or add to allowlist to allow all future PRs from the same author to run.

@AdamWill
Copy link
Contributor Author

I didn't run pre-commit because the dep chain is huge, sorry...

@AdamWill
Copy link
Contributor Author

Oh, forgot to mention, python/cpython#110274 is the reference for the exception type change upstream.

Copy link
Contributor

Unit Test Results

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

     15 files  ±0       15 suites  ±0   3h 24m 21s ⏱️ + 1m 34s
 13 123 tests ±0   12 158 ✅ ±0     931 💤 ±0  34 ❌ ±0 
162 520 runs  ±0  142 451 ✅  - 1  20 035 💤 +1  34 ❌ ±0 

For more details on these failures, see this check.

Results for commit 10f7321. ± Comparison against base commit f6e0690.

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.

Looks good. Thanks for taking care of this!

@fjetter fjetter merged commit 1755a71 into dask:main Jun 18, 2024
25 of 27 checks passed
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.

None yet

3 participants