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: loc-based indexing with float keys fails inconsistently if index is integer close to numeric limits #53290

Closed
2 of 3 tasks
wence- opened this issue May 18, 2023 · 14 comments
Labels
Bug Closing Candidate May be closeable, needs more eyeballs Indexing Related to indexing on series/frames, not to indexes themselves

Comments

@wence-
Copy link
Contributor

wence- commented May 18, 2023

Pandas version checks

  • I have checked that this issue has not already been reported.

  • I have confirmed this bug exists on the latest version of pandas.

  • I have confirmed this bug exists on the main branch of pandas.

Reproducible Example

import pandas as pd
s = pd.Series([1, 2, 3, 4], index=[2**63 - 1, 2**63 - 10, 2**63 - 100, 2**63 - 1000])

for i in [1, 10, 100, 1000]:
    index = 2**63 - i
    findex = float(index) # lossy
    for fidx, iidx in ((findex, index), ([findex], [index])):
        try:
            value = s.loc[fidx]
            print(f"Found {fidx} (2**63 - {i})")
            print(f"Got\n{value}")
            print(f"Expect\n{s.loc[iidx]}")
        except KeyError:
            print(f"Didn't find {fidx} (2**63 - {i})")

Issue Description

Running this produces:

Didn't find 9.223372036854776e+18 (2**63 - 1)
Found [9.223372036854776e+18] (2**63 - 1)
Got
9223372036854775708    3
dtype: int64
Expect
9223372036854775807    1
dtype: int64
Didn't find 9.223372036854776e+18 (2**63 - 10)
Found [9.223372036854776e+18] (2**63 - 10)
Got
9223372036854775708    3
dtype: int64
Expect
9223372036854775798    2
dtype: int64
Didn't find 9.223372036854776e+18 (2**63 - 100)
Found [9.223372036854776e+18] (2**63 - 100)
Got
9223372036854775708    3
dtype: int64
Expect
9223372036854775708    3
dtype: int64
Didn't find 9.223372036854775e+18 (2**63 - 1000)
Found [9.223372036854775e+18] (2**63 - 1000)
Got
9223372036854774808    4
dtype: int64
Expect
9223372036854774808    4
dtype: int64

When asking for a single scalar float, since the conversion is lossy, no .loc produces an answer (in the guts, there's an overflow-error when casting from the float back to an integer, since the nearest integer to float(2**63-1) is 2**63, similarly for -10 and -100, whereas float(2**63 - 1000) is representable as an integer, but is not 2**63 - 1000).

This is the behaviour I would expect, since the index does not contain the requested keys.

In contrast when indexing with a singleton list of the same values, indexing is always successful but only produces the correct answer for 2**63 - 100 and 2**63 - 1000. For 2**63 - 1 and 2**63 - 10 the entry for 2**63 - 100 is returned instead.

Aside: note that this is not symmetric: if the index is floats and we ask for an integer key with loc the behaviour is as if the key is cast to float first and then looked for.

Expected Behavior

Looking for a key that doesn't exist should consistently produce a keyerror. At the very least, these two broadly equivalent ways of indexing should have the same semantics.

FWIW, I would argue that the best thing is to deprecate lossy casting (e.g. cross-kind) in label-based indexing, and remove it in pandas 3. The semantics of the indexing are already hard enough to understand without having to have intimate knowledge of the behaviour of numpy's type promotion rules and their interaction with numeric representations. [Yes, I know that Python hashes small ints and small integral floats the same, I also think that was a bad decision]

Installed Versions

INSTALLED VERSIONS

commit : 37ea63d
python : 3.11.3.final.0
python-bits : 64
OS : Linux
OS-release : 5.19.0-41-generic
Version : #42~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue Apr 18 17:40:00 UTC 2
machine : x86_64
processor : x86_64
byteorder : little
LC_ALL : None
LANG : en_GB.UTF-8
LOCALE : en_GB.UTF-8

pandas : 2.0.1
numpy : 1.24.3
pytz : 2023.3
dateutil : 2.8.2
setuptools : 67.7.2
pip : 23.1.2
Cython : None
pytest : None
hypothesis : None
sphinx : None
blosc : None
feather : None
xlsxwriter : None
lxml.etree : None
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : None
IPython : 8.13.2
pandas_datareader: None
bs4 : None
bottleneck : None
brotli : None
fastparquet : None
fsspec : None
gcsfs : None
matplotlib : None
numba : None
numexpr : None
odfpy : None
openpyxl : None
pandas_gbq : None
pyarrow : None
pyreadstat : None
pyxlsb : None
s3fs : None
scipy : None
snappy : None
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : None
zstandard : None
tzdata : 2023.3
qtpy : None
pyqt5 : None

@wence- wence- added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels May 18, 2023
@wence-
Copy link
Contributor Author

wence- commented May 18, 2023

Another approach would be for the label-based indexing to map semantically onto the equivalent that would occur if the index were just a normal column and one were doing mask-based filtering (this would then use the normal cross-kind casting rules for binops).

@topper-123
Copy link
Contributor

Floats being lossy is well known and inherent to how computers work and things like this will happen when you use floats for indexing on integer indexes. So this isn't a bug, but a result of indexing with big floats on integer indexes (which you shouldn't do)

@topper-123 topper-123 closed this as not planned Won't fix, can't repro, duplicate, stale May 18, 2023
@topper-123 topper-123 added Indexing Related to indexing on series/frames, not to indexes themselves and removed Needs Triage Issue that has not been reviewed by a pandas team member labels May 18, 2023
@wence-
Copy link
Contributor Author

wence- commented May 18, 2023

So this isn't a bug, but a result of indexing with big floats on integer indexes (which you shouldn't do)

Is the pandas position therefore that indexing with floats on non-float indices is undefined behaviour?

@topper-123
Copy link
Contributor

topper-123 commented May 18, 2023

If the float and int evaluate equal, it's ok. The problem here is that 2**63 - 1 != float(2**63 - 1), so won't match in your case.

@wence-
Copy link
Contributor Author

wence- commented May 18, 2023

The problem here is that 263 - 1 != float(263 - 1), so won't match in your case.

Agreed, but I contend that it should consistently not match (note that .loc[[x]] and .loc[x] return different results in terms of whether they error or not).

@topper-123
Copy link
Contributor

That does look wrong to me. Can you open an issue on this?

@wence-
Copy link
Contributor Author

wence- commented May 19, 2023

I mean, that is this issue, no?

@topper-123
Copy link
Contributor

Oh, very sorry about that, I misunderstood the code snippet that you iterated first over ints then floats. I see now it's first scalars, then lists.

So the issue here is that lists of large floats work when indexing when they should fail? (while their scalar equivalent fails correctly?)

@topper-123 topper-123 reopened this May 19, 2023
@wence-
Copy link
Contributor Author

wence- commented May 24, 2023

So the issue here is that lists of large floats work when indexing when they should fail? (while their scalar equivalent fails correctly?)

Yes (for some definition of "works" that isn't the one I expected).

Just to clarify what is happening:

Index: [2**63 - 1, 2**63 - 10, 2**63 - 100, 2**63 - 1000]
Key: float(2**63 - 1)

s.loc[key] # => KeyError
s.loc[[key]] # => no error, returns single entry corresponding to `2**63 - 100`

I think there are few possible valid answers (depending on the casting rules for .loc):

  1. No casting, both indexing calls return KeyError
  2. Key is cast to dtype of index, lookup proceeds as normal (this would produce KeyError in this example)
  3. Key and Index are cast to common dtype, lookup proceeds as normal (this should, if the key is 2**63 - 1, produce a result that is the first three entries of the series, since 2**63 - 1, 2**63 - 10 and 2**63 - 100 all cast to the same float64.

I would say (1) is arguably the best option, but does introduce value-dependent KeyErrors (if the index is integral, and one looks up small floating point keys, everything works).

Hence, option (4): deprecate cross-kind casting in loc such that s.loc[1f] raises ValueError if s does not already have a float Index. (More radical, deprecate kind == f indices as well).

@wence-
Copy link
Contributor Author

wence- commented Jun 8, 2023

Any further thoughts here?

@topper-123
Copy link
Contributor

The underlying problems seems to be:

>>> big_int = 2**63 - 1
>>> big_float = float(big_int)
>>> np_float = np.array([big_int])[0]
>>>
>>> big_int == np_float
True
>>> big_float == np_float
True
>>> big_int == big_float
False

This is weird, the last statement isn't consistent with the two previous statements.

@wence-
Copy link
Contributor Author

wence- commented Jun 9, 2023

This is why cross-kind casting is so insidious (especially with operator overloading), and why I think that my option (4) [Disallow cross-kind casting in label-based indexing] above is the best long-term solution.

What is going on? These examples are all calling different routines, specifically:

  • big_int == np_float, lvalue is a PyLong, rvalue is a np.float64. so big_int.__eq__ returns NotImplemented and we call numpy's float64.__req__ method. So for the first thing you're effectively doing np.float64(big_int) == np_float.
  • big_float == np_float, lvalue is a PyFloat, rvalue is a np.float64 (for which PyCheck_Float is True, since numpy floats are subclasses of python floats). So this calls CPython's float_richcompare which compares two floats which are the same.
  • big_int == big_float, lvalue is a PyLong, rvalue is a PyFloat, big_int.__eq__ returns NotImplemented and we call CPython's float.__req__ method, which calls float_richcompare(big_float, big_int, __eq__). This does not perform kind-casting, but instead does value inspection to determine if the float and int really are equal, there's this comment at the top:
    /* Comparison is pretty much a nightmare.  When comparing float to float,
     * we do it as straightforwardly (and long-windedly) as conceivable, so
     * that, e.g., Python x == y delivers the same result as the platform
     * C x == y when x and/or y is a NaN.
     * When mixing float with an integer type, there's no good *uniform* approach.
     * Converting the double to an integer obviously doesn't work, since we
     * may lose info from fractional bits.  Converting the integer to a double
     * also has two failure modes:  (1) an int may trigger overflow (too
     * large to fit in the dynamic range of a C double); (2) even a C long may have
     * more bits than fit in a C double (e.g., on a 64-bit box long may have
     * 63 bits of precision, but a C double probably has only 53), and then
     * we can falsely claim equality when low-order integer bits are lost by
     * coercion to double.  So this part is painful too.
     */

@topper-123
Copy link
Contributor

Another way to see this problem is like this:

>>> s[[9223372036854775807]]
1
>>> s[[9223372036854775807.0]]
3

I don't think this is fixable and the lack of precision is inherent when using large floats.

@wence-
Copy link
Contributor Author

wence- commented Jun 12, 2023

OK, I've opened #53613 instead to propose fixing this by deprecating the behaviour completely.

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

No branches or pull requests

2 participants