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

BUG/TST: fix several issues with slice bound checking code #6531

Merged
merged 1 commit into from
Mar 5, 2014

Conversation

immerrr
Copy link
Contributor

@immerrr immerrr commented Mar 3, 2014

This patch removes more of what's left after removing pandas-specific customizations to slice indexing.

BUG/TST: obj.iloc[:,-4:-10] should return empty slice (because slice.step > 0 and slice.start > slice.stop)
There's no way to describe this by setting stop to a value in [0; len] interval, it should remain negative (see this ticket in python bugtracker]. Proposed fix to this issue is to remove the custom logic altogether. A side-effect of that is correct handling of negative steps in presence of out-of-bounds slices, e.g. obj.iloc[:,10:4:-1] would've incorrectly returned empty slice.

BUG: fix exception raised by Series([1,2,3]).iloc[10:].
After I've cleaned up suspicious code that handled empty-sequence keys in #6440, this piece of code started causing trouble:

if start >= l:
    return self._getitem_axis(tuple(),axis=axis)

CLN: #6299 & #6443 rendered core.indexing._check_slice_bounds function useless and it's now gone. Next step would be to drop unused raise_on_error parameter in slice-related functions.

Not sure, how do I put down all these relations into changelog, advice on that is welcome.

@jreback
Copy link
Contributor

jreback commented Mar 3, 2014

you can drop the raise_on_error parameter as it was just something to 'fix' the slicing issues (from somewhere in 0.12-0.13 transition).

Just put this all in a single line in API changes (its not really an API change, but more prominent), list all of the related issues.

@jreback jreback added this to the 0.14.0 milestone Mar 3, 2014
@immerrr
Copy link
Contributor Author

immerrr commented Mar 4, 2014

Just put this all in a single line in API changes (its not really an API change, but more prominent), list all of the related issues.

Did I get you correctly: immerrr@c7e6345#diff-5a809a5e093be4fe1396cbadd5bbeccaR110 ?

@jreback
Copy link
Contributor

jreback commented Mar 4, 2014

tep

@jreback
Copy link
Contributor

jreback commented Mar 4, 2014

yep

@immerrr
Copy link
Contributor Author

immerrr commented Mar 4, 2014

Ok, folding/rebasing it then.

@jreback
Copy link
Contributor

jreback commented Mar 4, 2014

minor change

I think you can move _slice from frame/panel to generic (but series still overrides)

@immerrr immerrr closed this Mar 5, 2014
@immerrr immerrr deleted the fix-out-of-bounds-slice-issues branch March 5, 2014 10:18
@immerrr immerrr restored the fix-out-of-bounds-slice-issues branch March 5, 2014 10:19
@immerrr
Copy link
Contributor Author

immerrr commented Mar 5, 2014

Whoops, apparently I've accidentally deleted wrong gh branch when cleaning up local ones, sorry about that.

@immerrr immerrr reopened this Mar 5, 2014
@immerrr
Copy link
Contributor Author

immerrr commented Mar 5, 2014

Also, _slice is now in ndarray as requested.

@jreback
Copy link
Contributor

jreback commented Mar 5, 2014

can you rebase on currentmaster and push again.thxs

BUG/TST: fix handling of slice.stop < -len, obj.iloc[:-len(obj)] should be empty

BUG/TST: fix exceptions raised by Series.iloc when slice.start > len

CLN: remove unused _check_slice_bound function and raise_on_error params
@immerrr
Copy link
Contributor Author

immerrr commented Mar 5, 2014

Ok, done.

jreback added a commit that referenced this pull request Mar 5, 2014
BUG/TST: fix several issues with slice bound checking code
@jreback jreback merged commit 549a390 into pandas-dev:master Mar 5, 2014
@jreback
Copy link
Contributor

jreback commented Mar 5, 2014

thanks as always!

if you are interestdd #6536

@immerrr immerrr deleted the fix-out-of-bounds-slice-issues branch March 5, 2014 11:41
@immerrr
Copy link
Contributor Author

immerrr commented Mar 5, 2014

Sure, that was a low hanging fruit :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants