Merged
Conversation
… and tests that verify the functionality. Current state: - a reference implementation for 1D slicing is implemented - a helper function that (so far) constructs a correct dask slice graph (minus step, indexing without slicing, and ellipsis) for n-dimensional slicing passes tests for 1D and 2D slicing Remaining to be written: - a front-end function (or perhaps a few functions) that setup the data necessary to correctly call the helper _dask_slice() Remaining to be figured out: - how to deal with slice steps in a clear, concise, easy-to-understand way - how to deal with indexing and ellipsis (array[0,0:4,...]) or open colon (array[:,0:3])
This is based on @mrocklin's ideas and prototyping for how things should work. The result is cleaner (so far). This implementation passes basic tests, and does not include any optimizations yet.
Adding comments and doctests revealed some bugs in the previous code that didn't properly handle steps with offsets. The updated code fixes these errors, but I'm not happy with it. I think that the code can be significantly simpler and clearer, but I don't understand it well-enough yet. As it stands, steps and starting offsets should be properly handled now.
These additions should better explain what and why the function. Added doctests to give examples of what/why should be returned.
- non-slice index examples added to doctest - Reformatted and updated docstrings - tested against python 3.4 and fixed integer vs. float division problems - reformatted to have 80 max 80 columns per line in code, and max 72 columns per line in comments Added optimization where if all slices are slice(None,None,None), the original object is returned.
Tested in both py2.7 and py3.4
Conflicts: dask/array.py
This resolves a core dump
This should complete dask block slicing. Many test cases have been added. Python 2 does not have an accumulate() in itertools, but python 3 does, so I added the reference implementation to compatibility.py
Added negative slicing capabilities to _slice_1d().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Smoothed out version of #9
Most work done by @nevermindewe