Skip to content

Support negative drop_axis in map_blocks and map_overlap#8192

Merged
jsignell merged 2 commits intodask:mainfrom
grlee77:drop-axis-allow-negative
Oct 1, 2021
Merged

Support negative drop_axis in map_blocks and map_overlap#8192
jsignell merged 2 commits intodask:mainfrom
grlee77:drop-axis-allow-negative

Conversation

@grlee77
Copy link
Copy Markdown
Contributor

@grlee77 grlee77 commented Sep 28, 2021

This PR allows the drop_axis argument of map_blocks and map_overlap to use negative axis values in the same manner as functions in the NumPy API.

This improves on #8064 which added a more informative error message on negative drop_axis (closing #7924). This PR changes the error to only occur when the specified axis exceeds the dimensions of the output array, otherwise the axis is converted to the equivalent positive-valued axis.

@GPUtester
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

@github-actions github-actions bot added the array label Sep 28, 2021
@jrbourbeau
Copy link
Copy Markdown
Member

add to allowlist

Copy link
Copy Markdown
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @grlee77! It looks like there are some code formatting issues -- see https://docs.dask.org/en/latest/develop.html#code-formatting for what formatting checks to run or how to (optionally) have these automatically run with pre-commit

@grlee77
Copy link
Copy Markdown
Contributor Author

grlee77 commented Sep 28, 2021

Thanks @grlee77! It looks like there are some code formatting issues -- see https://docs.dask.org/en/latest/develop.html#code-formatting for what formatting checks to run or how to (optionally) have these automatically run with pre-commit

Yeah, I had not installed pre-commit for this repo. It should be fixed now

@grlee77 grlee77 force-pushed the drop-axis-allow-negative branch from 8535ab1 to aa559ca Compare September 29, 2021 20:51
@jsignell
Copy link
Copy Markdown
Member

jsignell commented Oct 1, 2021

I'm going to re-run the windows tests; if they pass this is ready to go.

@jsignell jsignell merged commit e5bd5c4 into dask:main Oct 1, 2021
@jsignell
Copy link
Copy Markdown
Member

jsignell commented Oct 1, 2021

Thanks @grlee77 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants