Axis slicer
State: closed

The internal axis slicer. I overloaded `indexer` to be `start` when `stop` is passed in instead of having a separate indexer kwargs. 


Labels:
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
looks good....once travis finishes...can merge

Weird. One of the tests failed on plotting. All I did was have Travis rebuild and it worked. Shrug. 

Sent from my iPhone

> On Sep 26, 2013, at 10:04 AM, jreback notifications@github.com wrote:
> 
> looks good....once travis finishes...can merge
> 
> —
> Reply to this email directly or view it on GitHub.

ok...

I think @jtratner has a couple of question about some of the excetions..

@dalejung just left a bunch of comments. Basically, I want it to be clear that this is an internal function and callers should be checking for all those things. `assert` statements help make that clear that caller code should check this. (also, generally can phrase positively instead of negatively which is nice).  Finally, please note all the requirements you have in the docstring (ndim < 2, axis < ndim, must pass stop if indexer is None).

@jtratner I'm confused. I thought we had removed all `asserts` from core code a year ago since `assert` statements are removed when python is run with the optimize flag. 

@dalejung there were a lot of places using asserts incorrectly (like checking that input to `apply` was correct or something). But if this function is actually internal (i.e., everything that calls it is _supposed_ to check that the arguments are correct), then asserts make sense because they're doing exactly what they're supposed to be doing: checking that the internal logic is what you expect. (and if they ever bubble up to the user, that's an error in programming). Thus, it's okay if they are removed with `python -O` because they should _not_ be necessary for effective running of the program.

Moreover, any exception that this could raise wouldn't make sense to the user. What the heck does it mean to have this wrong start or stop or ndim or indexer? The caller needs to raise a more useful error to the user.

How is an AssertionError, with the same message, any more understandable by an end user? 

If `python -O` is run, doesn't it make more sense to `raise` than to silently return bad data and have it possibly error in some less useful place? Or even worse, follow the chain and return bad data? There is one `assert` statement in `core` and it was put in there 10 days ago. 

@dalejung it's not, that's why caller functions have to check for these conditions first. The point is that none of those checks are necessary, because callers need to check for it - you're just establishing the logic that you think should be the case when this function is called. The reason that `python -O` strips out asserts is that those statements are supposed to be unnecessary. (which, again, they are going to be, because callers have to check it).

the entirety of the change with -O is that asserts are stripped - http://stackoverflow.com/questions/2830358/what-are-the-implications-of-running-python-with-the-optimize-flag , there's no benefit to it other than that.

@dalejung last comment on this from me: the reason that all the asserts were changed to `raise AssertionError` is because we weren't clear which were "internal" asserts (i.e., all callers check that the input is correct) vs. errors validating user input (which need to stay in regardless). Anything that validates user input will _eventually_ be changed to a different kind of error because AssertionError is the wrong type. Those that still remain we still haven't gotten to yet.  `AssertionError`s that have a `# pragma: no cover` with them could be changed to asserts because it it (should be) impossible to get to that state from the public API.

Anyways, if you don't agree with me on this, then I'll just agree to disagree with you.  Then you'd just need to choose the right exceptions to raise (can't raise `Exception` - it's bad style and hard to catch later) and, for each exception, it would be good to add a test case that triggers it from the _public_ API.

@jtratner Well, I think I can actual resolve this. From the original comment about it on #4874:

> It's just a small util I've been using when generating tests, but _I'm not sure if it should go into pandas_ and if so where.

I actually use it external to pandas core development. It came out of a private repo. So it's only quasi-internal. Personally, I use a lot of "internal" functions from `pandas` like the low level binning. And outside of me, who's to say other people might not use a _convenience_ function in their own libs. 

I'll update the exceptions.

@dalejung @jtratner mergeable?

@jreback Not atm. Need to update exceptions. Got sidetracked on other work.

@dalejung ok...np...I believe you need this before the Panel.shift.

ping when ready

@dalejung ?

@dalejung ?

@dalejung was accid closed, but needs to revisit this in any event

pls rebase this....this would be nice as an internal function

@dalejung this is actually a nice, PR, pls reopen/resubmit when ready