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

Use `asarray` in `extract` #4549

Merged
merged 1 commit into from Mar 6, 2019

Conversation

Projects
None yet
3 participants
@jakirkham
Copy link
Member

commented Mar 6, 2019

Force both array inputs to Dask Arrays. This avoids potentially converting a different array type to a NumPy array by accident.

  • Tests added / passed
  • Passes flake8 dask
Use `asarray` in `extract`
Force both array inputs to Dask Arrays. This avoids potentially
converting a different array type to a NumPy array by accident.

@jakirkham jakirkham force-pushed the jakirkham:use_da_asarray_extract branch from 6cabfbd to 27f8926 Mar 6, 2019

@mrocklin

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

This looks fine to me. I wonder how we should end up testing changes like this.

Something with sparse perhaps?

(not a requirement for this PR necessarily, just wondering what the right approach is long term)

@mrocklin

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

Also cc @pentschev , just so that he's aware

@jakirkham

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2019

It's a fair question. Though it's probably one that we want to ask over the test suite as a whole as opposed to each test case. Pytest's fixtures come to mind, but IDK if that is the right solution. Should we move this to an issue to discuss further?

@mrocklin

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

@pentschev

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

I think it would be good indeed to have more tests with Sparse too, in particular for these issues related to NEP-18. I'll try to add some more within the context of #4486.

@mrocklin mrocklin merged commit bb6b5a9 into dask:master Mar 6, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jakirkham jakirkham deleted the jakirkham:use_da_asarray_extract branch Mar 6, 2019

jorge-pessoa pushed a commit to jorge-pessoa/dask that referenced this pull request May 14, 2019

Use `asarray` in `extract` (dask#4549)
Force both array inputs to Dask Arrays. This avoids potentially
converting a different array type to a NumPy array by accident.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.