BUG/ENH: provide better .loc based semantics for float based indicies, continuing not to fallback (related GH236)
State: closed

closes #236 
- CLN: refactor all scalar/slicing code from core/indexing.py to core/index.py on a per-index basis
- BUG: provide better `.loc` semantics on floating indicies
- API: provide a implementation of `Float64Index`
- BUG: raise on indexing with float keys on a non-float index

provide better `loc` semantics on `Float64Index`

```
In [1]: s = Series(np.arange(5), index=np.arange(5) * 2.5)

In [9]: s.index
Out[9]: Float64Index([0.0, 2.5, 5.0, 7.5, 10.0], dtype=object)

In [2]: s
Out[2]: 
0.0     0
2.5     1
5.0     2
7.5     3
10.0    4
dtype: int64

In [3]: # label based slicing

In [4]: s[1.0:3.0]
Out[4]: 
2.5    1
dtype: int64

In [5]: s.ix[1.0:3.0]
Out[5]: 
2.5    1
dtype: int64

In [6]: s.loc[1.0:3.0]
Out[6]: 
2.5    1
dtype: int64

In [7]: # exact indexing when found

In [8]: s[5.0]
Out[8]: 2

In [9]: s.loc[5.0]
Out[9]: 2

In [10]: s.ix[5.0]
Out[10]: 2

In [11]: # non-fallback location based should raise this error (__getitem__,ix fallback here)

In [12]: s.loc[4.0]
KeyError: 'the label [4.0] is not in the [index]'

In [13]: s[4.0] == s[4]
Out[13]: True

In [14]: s[4] == s[4]
Out[14]: True

# [],ix,s.ix[2:5]
loc is clear about slicing on the values ONLY

In [11]: s[2:5]
Out[11]: 
2.5    1
5.0    2
dtype: int64

In [12]: s.ix[2:5]
Out[12]: 
2.5    1
5.0    2
dtype: int64

In [2]: s.loc[2:5]
Out[2]: 
2.5    1
5.0    2
dtype: int64

In [15]: s.loc[2.0:5.0]
Out[15]: 
2.5    1
5.0    2
dtype: int64

In [16]: s.loc[2.0:5]
Out[16]: 
2.5    1
5.0    2
dtype: int64

In [17]: s.loc[2.1:5]
Out[17]: 
2.5    1
5.0    2
dtype: int64
```

Float Slicing now raises when indexing by a non-integer slicer

```
 In [1]: s = Series(np.arange(5))

In [2]: s
Out[2]: 
0    0
1    1
2    2
3    3
4    4
dtype: int64

In [3]: s[3.5]
KeyError: 

In [4]: s.loc[3.5]
KeyError: 'the label [3.5] is not in the [index]'

In [5]: s.ix[3.5]
KeyError: 'the label [3.5] is not in the [index]'

In [6]: s.iloc[3.5]
TypeError: the positional label [3.5] is not a proper indexer for this index type (Float64Index)
```


Labels:

Comments:
The behaviour in the test case does look like what I'm hoping for, though.

updated....

unfortunately not sure what if anything to do about **getitem** and ix for a floating index...hmm

For **getitem** and ix, one thing to do is to fix an independent bug that
forms a superset of this one: that when doing offset indexing, you
shouldn't call int(), you should call .**index**(). float.**index** raises
an error, so this would at least tell us that s.ix[4.0] and s.ix[4.0:5]
can't be valid offset indexes. Notice that right now:

In [20]: pandas.Series(np.arange(5))[3.5]
Out[20]: 3

Using .**index** is how Python indexing works in general (e.g., what lists
and tuples do), and it was actually added to Python aeons ago specifically
so numpy would have a solid standard to be compatible with. Sadly numpy's
only now in the process of switching to using this. In 1.8, doing something
like np.arange(5)[3.5] will raise a DeprecationWarning, and then in 1.9 or
1.10 or so it will become an error. But pandas could probably get ahead of
the game here if it wanted to. (And should probably keep up in any case!)

That does still leave the question of what to do with s.ix[4:5] when
there's a floating point index which implies that in principle this could
always refer to a label-based span. But I guess for backwards compatibility
you have to leave that as doing offset-based indexing, so that's that?

On Mon, Sep 16, 2013 at 6:24 PM, jreback notifications@github.com wrote:

> updated....
> 
> unfortunately not sure what if anything to do about _getitem_ and ix for
> a floating index...hmm
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/4850#issuecomment-24527821
> .

@njsmith updated...take a look

I stub-implemented Float64Index (which means its just a new class, but is still dtype=object)

@njsmith 

its pretty easy to make `s.ix[2:5]` be the same as `s.ix[2.0:5]`...e.g. make it a float selection
as this is very rational

however `s[2:5]` will be a positional selection, UNLESS since I know this is a float index we make these do slicing selection on the axis (e.g. NO FALLBACK)

If I basically eliminate fallback, the all 8 of these cases will return the same.

that seems good (and this will ONLY happen for a float based index).

its an API change, but I don't think too big

thoughts?

@njsmith 

I have removed the fallback stuff, will have to revist at some point. I can take it out, but then semantics become very weird.

e.g.

`s[4]` or `s[4.0]` should do what? KeyError because the float is not there (which is what I think it should do), but
actually making that happen is a bit non-trivial.

I think what exists now does a pretty good job via `.loc`. (and of course you have `.iloc`), so you can decide what you want.

With a `Float64Index` **getitem** and `.ix` just have different (and weird) semantics depending on what you are doing. avoid them, until/unless we drop fallback indexing (on FloatIndexes) entirely.

But that will be another issue.

Sorry, with all this back and forth I've gotten pretty lost, and I'm finding your notes just a hair too telegraphic to really follow. Would it be possible to summarize exactly what behavioural changes are now included in this PR?

@njsmith 

I updated the header of this PR

3 changes (which are in order)
- BUG: provide better .loc semantics on floating indicies
- API: provide a stub-like implementation of Float64Index
- BUG: raise on indexing with float keys on a non-float index

Sorry, not trying to be dense, but still pretty confused.
- What are the "better" `.loc` semantics that you settled on in the end?
- Does the addition of the Float64Index create any user-visible behavioural changes, or is it just laying the groundwork for future work?
- From the tests I had the impression that you changed how `.ix` and `.__getitem__` work with floating indexes, but according to your summary now, you didn't touch these. Which is correct?

look at the first example in the top of the PR, `loc` works under all slicing regimes to give the same result

```
# loc is clear about slicing on the values ONLY

In [2]: s.loc[2:5]
Out[2]: 
2.5    1
5.0    2
dtype: int64

In [15]: s.loc[2.0:5.0]
Out[15]: 
2.5    1
5.0    2
dtype: int64

In [16]: s.loc[2.0:5]
Out[16]: 
2.5    1
5.0    2
dtype: int64

In [17]: s.loc[2.1:5]
Out[17]: 
2.5    1
5.0    2
dtype: int64
```

This is the 'same' as `.ix` but doesn't have fallback, that's the difference
- `Float64Index` now exists, but its implementation is the same. Haven't really 'changed' anything
  on it yet, so it is laying future groundwork
- no changes to `[]` nor `ix`, these are tested elsewhere in full (I put in some tests, but commented them out) in the current version as they don't have the _correct_ semantics for `Float64Index`....I have a branch to 'fix' it, but seemed like too much of a change (and there were some loose ends), so I took it out.

essentially I CAN remove fallback indexing entirely from `[]` and `ix` (so in reality `ix` would be the same as the new `loc`) FOR `Float64Index` - this will have to be a different PR.

I believe the PR as is solves all of the issues that you brought up. You can simply use `loc` for all of your indexing needs with `Float64Index` and it will do what you expect. There still will remain idiosyncrasies with `[]` and `ix` wrt `Float64Index`, but as I said above, too much changing at once.

Okay, got it now.

The `.loc` changes for floating point indexes look good to me.

The handling of the float truncation issue is still a bit wonky. You aren't testing what happens with a non-float-index Series, or with .iloc, when someone puts floats into a fancy index (list/ndarray) or slice, and I'm pretty sure at least slices will still do truncation in this case. And your code can't possibly be fully correct for integer indexing if it never refers to `.__index__`. But it does look better than it was!

BTW this stuff in `_is_index_slice` that checks for floats being near integers looks dubious to me given these other changes, but not sure.

@njsmith this whole fallback notion is wonky! (but is somewhat necessary). Its just that for a Float64Index, I think you never want fallback. Problem is only some of the logic is actually implemented thru the index, some is directly on **getitem** and such. You are welcome to take a stab (on top of this PR or independently) if you like. It is quite easy to break something while fixing another problem. Its like whack a mole.

what would be appreciated is a larger set of tests with various kinds of indicies and the expections give various kinds of inputs.

test_indexing does quite a lot of this, but apparently some cases were missed.

All I'm saying with the "wonky" comment is that the behaviour of `Series(np.arange(5))[3.5]` should probably be consistent with the behaviour of `Series(np.arange(5))[3.5:4.5]`. This part has nothing to do with float indices, it just looks to me like an obvious inconsistency/bug in the PR.

there are some inconsistenties...trying to nail them down...but this is quite complicated!

Yeah, I hear that!

Another minor thought that just occurred to me while writing some code that instantiates these: it would be nice if explicitly calling the `Float64Index` constructor would guarantee that you actually get a `Float64Index` (and error out otherwise). Anyone who takes the trouble to call it directly probably is doing so b/c they want to make sure that they're getting its special behaviour. (Of course in my code for now I can't assume `Float64Index` exists so I'll just trust that using an ndarray with float64 dtype will do the job, which I think is right. But still, it'd be nice.)

`s = Series([0,1,2],['foo','bar'','bar'])`

what should this yield for (this is why this is tricky) as you can make a case that `s[2.0]` shouln't work
at all, but at the same time when do you decide that

```
s[2]
s[2.0]
s.loc[2]
s.loc[2.0]
s.ix[2]
s.ix[2.0]
s.iloc[2]
s.iloc[2.0]
```

in answer to your question, `Float64Index` is still dtype == 'object', but will be inferred if you have at least 1 floating point value (or the dtype of the index is `np.float`)

Since there's obviously no label-based indexing going on here, we only have
to consider what should happen for integer/offset-based indexing.

The general python rule _for integer/offset-based_ indexing (used by e.g.
lists), and the one that numpy will switch to in 1.9 or so, is that using
an object with type 'float' is _always_ an error, no matter whether this
float happens to have an integer value. (Or more specifically, the rule is
that an object must either be of type 'int', or else it must have a
.**index**() method which returns an int.) So every example there with
[2.0] should become an error eventually, possibly after a deprecation
period.

The one other questionable case for me is s.loc[2]. I actually think this
should be an error -- currently .loc's logic is quite confusing and
.ix-like, where when it sees an integer it uses some heuristics to guess
whether this is a label or an offset. AFAICT the rule right now is:
- if you have an Int64Index, then that means that all labels are integers,
  and THEREFORE all integers passed to loc must be labels.
- if you have a generic Index, then your labels could be _any_ random
  (hashable) python objects; and in particular, they might or might not be
  integers, and THEREFORE integers passed to loc might or might not be labels.

The problem is that when you actually lay out the logic like this you can
see that those two THEREFOREs are completely invalid conclusions, logically
speaking :-). Straight up post hoc ergo propter hoc.

IMHO to make .loc predictable, the fallback to integer/offset-based
indexing should only be allowed if the Index can guarantee that _no_ labels
are integers. (And by "integers" we mean, compare == to a Python integer.
Which Python floats can do.) So that would mean that .loc ought to be able
to do integer/offset-based indexing for DatetimeIndex and MultiIndex, but
not Index or Int64Index or Float64Index.

In practice I guess the only _common_ situations where people use
plain-old-generic 'Index' right now, once Float64Index is added, is for
all-string indexes. If the above rule is adopted -- the one that says for a
generic Index that can contain anything, .loc must _always_ be label based,
never fall back to being integer/offset-based -- then people using
all-string indexes would probably be happier if a specialized StringIndex
was created that relaxed this rule.

On Wed, Sep 18, 2013 at 8:48 PM, jreback notifications@github.com wrote:

> s = Series([0,1,2],['foo','bar'','bar'])
> 
> what should this yield for (this is why this is tricky) as you can make a
> case that s[2.0] shouln't work
> at all, but at the same time when do you decide that
> 
> s[2]
> s[2.0]
> s.loc[2]
> s.loc[2.0]
> s.ix[2]
> s.ix[2.0]
> s.iloc[2]
> s.iloc[2.0]
> ``
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/4850#issuecomment-24693759
> .

Shouldn't the rule be, Float64Index is inferred if you have _all_ floating
point values? (Or maybe some integers mixed in -- np.asarray([1.0, 2])
upcasts the integer to a float, which is fine.)

Basicaly Index([1.0, "hi", None]) should not return a Float64Index, is what
I'm saying :-).

On Wed, Sep 18, 2013 at 8:49 PM, jreback notifications@github.com wrote:

> in answer to your question, Float64Index is still dtype == 'object', but
> will be inferred if you have at least 1 floating point value (or the dtype
> of the index is np.float)
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/4850#issuecomment-24693824
> .

no...`.loc` for both cases will error (now and in 0.12), ALWAYS, that's the point, it NEVER does fallback at ALL

and can't just give up on floating point, e.g. 2.0 as that's the default now

Oh, well, I actually like the rule that s.loc[2] should always error out
(unless there is an item labeled 2 in the index), and certainly if that's
the idea then I was overthinking it.

But that's not actually how it works in my 0.12.0:

In [6]: pandas.**version**
Out[6]: '0.12.0'

In [7]: s = pandas.Series([0,1,2],['foo','bar','bar'])

In [8]: s.loc[2]
Out[8]: 2

AFAICT if you have an Int64Index, then .loc acts the way you're saying, and
if you don't, then .loc will fallback. That's what I was basing my analysis
above on.

On Wed, Sep 18, 2013 at 9:16 PM, jreback notifications@github.com wrote:

> no....loc for both cases will error (now and in 0.12), ALWAYS, that's the
> point, it NEVER does fallback at ALL
> 
> and can't just give up on floating point, e.g. 2.0 as that's the default
> now
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/4850#issuecomment-24695671
> .

And on the floating point thing -- not sure what you mean by "giving up on
floating point". If you're just saying that we have to be careful about
changing how .ix and **getattr** work because of compatibility stuff, then
that's fine with me... that's why it's going to take numpy a few releases
before it can make np.arange(10)[2.0] into an error. But it will eventually.

On Wed, Sep 18, 2013 at 9:16 PM, jreback notifications@github.com wrote:

> no....loc for both cases will error (now and in 0.12), ALWAYS, that's the
> point, it NEVER does fallback at ALL
> 
> and can't just give up on floating point, e.g. 2.0 as that's the default
> now
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/4850#issuecomment-24695671
> .

@njsmith actually that `.loc[2]` is a bug (separate).....way too many cases here

@njsmith no that's exactly the point

currently `s[2]` and `s[2.0]` work. The argument is that the 2nd should NOT work (all for that), but currently
it does and pretty hard to fix w/o dropping support entirely.

indexing (probably historical) has lots and lots of cases. 

Okay, yeah.

So you asked about what [2.0] and [2] should do in all the various cases
for s = Series([0,1,2],['foo','bar','bar'])

Let me try and sum up my answer now that we're maybe on the same page :-)

1) All of the [2.0] cases should eventually be errors. When and how this
happens is up to you guys, but that's a release engineering problem, not a
conceptual design problem, so not _my_ problem ;-).
2) IF .loc is supposed to fall back to integer indexing in some cases, it
shouldn't do that for a plain-old-Index.
3) OTOH, IF .loc is _not_ supposed to fall back to integer indexing, then
obviously .loc[2] should be an error.

Summing up: assuming that .loc is never supposed to fall back, then here's
what ought to happen in each case:

s[2]        # ok
s[2.0]      # error (someday)
s.loc[2]    # error
s.loc[2.0]  # error
s.ix[2]     # ok
s.ix[2.0]   # error (someday)
s.iloc[2]   # ok
s.iloc[2.0] # error (someday)

Does that help...?

On Wed, Sep 18, 2013 at 9:33 PM, jreback notifications@github.com wrote:

> @njsmith https://github.com/njsmith no that's exactly the point
> 
> currently s[2] and s[2.0] work. The argument is that the 2nd should NOT
> work (all for that), but currently
> it does and pretty hard to fix w/o dropping support entirely.
> 
> indexing (probably historical) has lots and lots of cases.
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/4850#issuecomment-24696904
> .

@njsmith ok....I think I have tests for all scalar indexing (and as above should be case), except the somedays are right now working (e.g. there is fallback on `ix/[]` (the `iloc` doesn't fallback so much as allow a convert of `2.0` to `2`)

maybe eventually eliminate that (as you suggest)

@njsmith 

major effort here!

for net-net not a whole lot of outward changes, except now float indexing works pretty nicely
read the docs and try out; whole lot of tests

still a couple of warts, e.g. `iloc` coerces exact floats to ints rather than raise in slicing as does `[]/ix` for scalar accessors, but these are pretty minor

float indexes should be pretty predictible as slicing will ALWAYS be on the values for `[],ix,loc`, while `iloc` ALWAYS is positional

other types of indexing are pretty unaffected (except some more `TypeError` in invalid slicing/scalar indexing cases)

whew!

Wow, I'll take a look!

Okay, think that's all my minor comments. This is awesome and much appreciated!

My one major question is this: having slept on it, I find myself coming around to your argument yesterday that on `Float64Index`, `__getitem__` and `.ix` should _never_ do fallback. There's precedent for this in that `Int64Index` never does fallback for `__getitem__` and `.ix`, and `Float64Index` and `Int64Index` seem like they should behave the same way here -- in both cases what the index class is telling us is that integers are definitely the sort of thing that's being used for labels. (And in `Float64Index`'s case, non-integer floats as well, but that's not really relevant either way to the logic of whether one should do fallback AFAICT.) Also, I'm uncomfortable with the idea that `s[5]` and `s[5:6]` have completely different indexing semantics. And, most importantly, it would guarantee predictable behaviour for things like `.ix[5.0]` -- in the long run like we discussed this will probably never allow fallback, but it'd be nice to get the guarantee early for `Float64Index`, since `Float64Index` is exactly the case where floats falling back to integer indexing are going to be the most confusing and dangerous. Turning off fallback entirely would accomplish that. So... what do you think? I know it's more work, of course :-/

One more comment: once we've nailed down scalars and slicing, it'd probably be a good idea to go through and add some tests to make sure that fancy-indexing stuff like `s[[5, 6.5, 7]]` is treated consistently with scalars, just to avoid another round of weird surprises later. Intuitively, for all scalars `x`, `s[[x]]` should always give you the same thing as `s[x]` but with 1 extra dimension. So the tests should be pretty easy to generate?