dask
dask copied to clipboard
Add array annotations
Adding annotations to array functions. Primary motivation for this PR is improving compatibility with projects that use PyRight (a popular type checker for Python).
This PR adds type annotations for:
from_arrayrechunkmap_blocksmap_overlap
In particular, adding annotations to map_blocks yields a return type of class <method> for the method. @sam-goodwin if possible, please advise on whether this solves your problem with Pyright!
I have two main questions for this PR:
- Approach Much of the existing code was not written with types in mind. Adding types causes a lot of mypy errors. I've chosen to ignore those errors. Do admins think this is a bad idea?
Making the code type-safe would involve refactoring these functions. For example it's common to see:
- Variable is declared as a parameter with an expected type
- Variable is reassigned to an object of a different type for use throughout the function
This pattern causes mypy some distress!
For this PR, I decided to add type annotations, and ignore mypy warnings where necessary. This does lead to a lot of type: ignore statements. However I believe this is the best option because refactoring this code could be pulling on a long thread.
A refactor would delay deliver of a feature someone is asking for, only to fix something that does not seem to be a problem for most users. Plus, accurate type annotations will actually help users avoid type-unsafe operations in the first place. But I am curious to hear an admin's opinion on this.
- Type choices I did my best to choose types based on the docstrings and code context. I'd appreciate a once-over from someone who's familiar with these functions!
- [ ] Closes #11047
- [ ] Passes
pre-commit run --all-files
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.
Digging into this, it's possible that mypy does not like my choice of annotations. I'll keep looking into it.
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 27m 45s :stopwatch: - 1m 11s 13 127 tests + 1 12 012 :white_check_mark: +15 1 068 :zzz: ±0 47 :x: - 14 162 771 runs +12 139 937 :white_check_mark: +26 22 787 :zzz: ±0 47 :x: - 14
For more details on these failures, see this check.
Results for commit 4d04c9f5. ± Comparison against base commit b4b33cae.
:recycle: This comment has been updated with latest results.
Just seen this is still open 🥲 any chance one of the maintainers is able to give it a quick once over?
It's only type annotations so isn't that complex a merge! (plus would close an issue!)
(hope this bump isn't too rude- I know you guys are all pretty busy!!)
Hello, just wanted to check in and see if this got any attention. I moved to fully typed code in my latest project and dask was the only library with significant lack of annotations. I really want to give this issue a bump. Installing this branch manually solved all my issues, so props to the author(s).