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

Only call frame_split_size when there are frames #3996

Merged
merged 2 commits into from
Jul 31, 2020

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Jul 28, 2020

This just becomes a no-op if an empty list is given (returning an empty list). However there is no need to do this work if we don't need it. So simply restrict calling frame_split_size to the case where frames are present and compression will occur.

This just becomes a no-op if an empty list is given (returning an empty
list). However there is no need to do this work if we don't need it. So
simply restrict calling `frame_split_size` to the case where frames are
present and compression will occur.
@jakirkham jakirkham force-pushed the use_frame_split_size_as_needed branch from 3477cd0 to 4912667 Compare July 28, 2020 03:21
@quasiben
Copy link
Member

Ive been seeing the test_bandwidth in a few other PRs. Would you mind merging with master and resubmitting ?

@jakirkham
Copy link
Member Author

Unfortunately this is already based on the latest commit in dask/master.

@quasiben
Copy link
Member

I restarted CI to see if that fixes things

@jakirkham
Copy link
Member Author

Yeah test_bandwidth also fails on master. Submitted PR ( #3999 ) in an attempt to fix this.

@quasiben
Copy link
Member

Looks good. Thanks @jakirkham

@quasiben quasiben merged commit 80ea63a into dask:master Jul 31, 2020
@jakirkham jakirkham deleted the use_frame_split_size_as_needed branch July 31, 2020 17:44
@jakirkham
Copy link
Member Author

Thanks Ben 🙂

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.

2 participants