Skip to content

Set "priority" annotations in SimpleShuffleLayer-based __init__ functions#7846

Merged
mrocklin merged 9 commits into
dask:mainfrom
rjzamora:set-split-priority
Jul 1, 2021
Merged

Set "priority" annotations in SimpleShuffleLayer-based __init__ functions#7846
mrocklin merged 9 commits into
dask:mainfrom
rjzamora:set-split-priority

Conversation

@rjzamora

@rjzamora rjzamora commented Jun 29, 2021

Copy link
Copy Markdown
Member

Possible alternative to #7826

@rjzamora

Copy link
Copy Markdown
Member Author

@mrocklin - Is this what you had in mind? If not, I'll be happy to revise.

Comment thread dask/dataframe/tests/test_shuffle.py Outdated
@rjzamora
rjzamora marked this pull request as draft June 29, 2021 20:24

@madsbk madsbk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @rjzamora nice work. I agree with @mrocklin, it would be good with a more general test but I don't think you should put too much work into it.

Comment thread dask/layers.py Outdated
@rjzamora

Copy link
Copy Markdown
Member Author

it would be good with a more general test but I don't think you should put too much work into it.

I actually started looking into this, and I am not convinced the current version of this PR is "working" - So I do plan to investigate a bit more today.

@mrocklin

mrocklin commented Jun 30, 2021 via email

Copy link
Copy Markdown
Member

Comment thread dask/tests/test_distributed.py Outdated
@rjzamora
rjzamora marked this pull request as ready for review June 30, 2021 14:51
Comment thread dask/tests/test_distributed.py Outdated
@mrocklin

mrocklin commented Jul 1, 2021

Copy link
Copy Markdown
Member

@madsbk @rjzamora which is better this or #7826 ? I'm happy with either. We should get one in.

@rjzamora

rjzamora commented Jul 1, 2021

Copy link
Copy Markdown
Member Author

which is better this or #7826 ? I'm happy with either. We should get one in.

I have no real preference - If you prefer #7826, I suggest adding the test from this PR

@madsbk

madsbk commented Jul 1, 2021 via email

Copy link
Copy Markdown
Contributor

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.

4 participants