Conversation
jrbourbeau
left a comment
jrbourbeau
left a comment
There was a problem hiding this comment.
Nice! Thanks for taking this on. I see this is marked as a draft PR -- just let me know when you'd like a review
|
I opened up a similar PR doing this for distributed #4600 - tried to keep my settings consistent with those used here, feel free to let me know if you think some things should be different for Distributed! |
|
Thanks for the ping @charlesbluca! I ran into a bunch of issues after running isort on dask and haven't come back to it. Please let me know if you want to chat or come up with any best practices. |
|
No problem! Out of interest, were the issues flake8 errors when running pre-commit or did tests start failing after the resort? I don't have much experience with package sorting, so from my perspective things look good as is, but I'm happy to discuss any tweaks that might be needed either on here or Gitter (or elsewhere). |
|
Tests started failing after the resort. I think it probably had to do with how dask handles optional dependencies. I'll give it another go next week. |
|
Ok the last 3 commits actually do the isort. It'd be good to merge this quickly if people are happy with it, otherwise I will revert and rebase if main drifts. |
|
|
||
|
|
||
| from .utils import meta_from_array, compute_meta | ||
| from .blockwise import blockwise |
There was a problem hiding this comment.
I moved this to the bottom of the file to avoid circular dependencies. This is the only change that I made to what isort did by default. @jrbourbeau
jrbourbeau
left a comment
jrbourbeau
left a comment
There was a problem hiding this comment.
Thanks @jsignell! I've left a couple of small comments, otherwise this looks great -- thanks for taking this on
What do you think about treating distributed as a special third-party package that appear right above the dask imports instead of being grouped with things like numpy, cloudpickle, etc.?
diff --git a/setup.cfg b/setup.cfg
index c44894ae..5ccd68c4 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -18,11 +18,13 @@ ignore =
max-line-length = 120
[isort]
+sections=FUTURE,STDLIB,THIRDPARTY,DISTRIBUTED,FIRSTPARTY,LOCALFOLDER
profile = black
skip_gitignore = true
force_to_top = true
default_section = THIRDPARTY
known_first_party = dask
+known_distributed=distributed
[versioneer]
VCS = git
|
@jrbourbeau are you happy for this to merge if it passes? |
jrbourbeau
left a comment
jrbourbeau
left a comment
There was a problem hiding this comment.
Yeah, thanks for working on this @jsignell
3 participants
black dask/flake8 dask