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

Array chunk warning #6872

Closed
wants to merge 4 commits into from
Closed

Conversation

mattchurgin
Copy link

@mattchurgin mattchurgin commented Nov 21, 2020

  • [] Tests passed
  • Passes black dask / flake8 dask

Pull request to address issue #6810 . Issue a warning when mapping a function with 'axis' arguments onto a dask array. This alerts users to possible unexpected behavior when applying a function with axis argument along a dask array axis comprised of more than 1 chunk. Unfortunately not all tests are passing (they passed in my env before this commit). @jsignell would you mind taking a look when you have a chance and seeing if my approach to warning makes sense?

Copy link
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

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

This seems like a nice addition, but you'll need to make sure that the existing tests pass and add a new test to check this new functionality.

dask/array/core.py Show resolved Hide resolved
dask/array/core.py Outdated Show resolved Hide resolved
dask/array/core.py Outdated Show resolved Hide resolved
@mattchurgin
Copy link
Author

Thanks @jsignell for the feedback. I incorporated it and added a test.

I saw some tests were failing, and it looks like that is because the default behavior of pytest here is to throw an error on warnings. When I run pytest with -W ignore::UserWarning, all my tests pass. All tests also pass when I comment out all of my additions except for the warning. I'm not sure if it's possible/reasonable to include an ignore::UserWarning in the pytest config. I'm actually also not sure where pytest is configured in the codebase. What do you think?

@jsignell
Copy link
Member

I think you'd want to treat each test separately by adding a decorator like @pytest.mark.filterwarnings("ignore:Operating along axis with nchunks>1").

But having seen how much this pattern shows up in the test codebase, I am unsure whether we really want this warning or not (sorry!). I think if there were a way to disable it or an existing pattern of enabling more warnings it could be really nice to have this kind of thing. Maybe someone can chime in with a second opinion? @dask/maintenance

@mattchurgin
Copy link
Author

Thanks @jsignell for the feedback. I appreciate it. Happy to continue working on this if people think it's valuable. I see from the tests that it's assumed users have a good understanding of how the chunk structure of their array influences how computations on it will behave. At the same time, not everyone will do that. I agree we don't want to overdo it with warnings. I leave it up to you and the maintainers to decide whether this use case is overdoing it. Thanks!

@jsignell
Copy link
Member

Ok I am going to close this - I think we want to err on the side of only really important warnings. Thank you for opening it though!

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.

None yet

2 participants