-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
absolufy-imports - No relative - PEP8 #8796
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
Conversation
jcrist
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay for automating away an opinion-based choice! I habitually use relative imports (they used to be the main recommendation), but habits can change.
Would like to make sure we have consensus-ish across dask + distributed before merging this though.
|
could you fix merge conflicts? |
2326db2 to
a6d6e41
Compare
|
Thanks for reviewing @crusaderky! Let me know if you notice any other patterns that are wierd. |
|
I'm fixing conflicts then going to merge. |
a6d6e41 to
44092ec
Compare
|
Thanks Julia! 😄 |
|
@dask/maintenance if you see lots of conflicts it's likely to be this 😬 |
Updated imports to be compatible with latest dask after dask/dask#8796 This change is also compatible with dask versions prior to the above dask PR. Tested `import dask_cudf` with both dask `2022.02.2a220314` and `2022.02.2a220315`. Authors: - Rick Ratzel (https://github.com/rlratzel) Approvers: - Ashwin Srinath (https://github.com/shwina) URL: #10442
Matches dask/distributed#5924
Conversation in dask/distributed#5889