Skip to content

Remove Index.__inv__ #22335

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

Closed
toobaz opened this issue Aug 14, 2018 · 6 comments
Closed

Remove Index.__inv__ #22335

toobaz opened this issue Aug 14, 2018 · 6 comments
Labels
Bug Clean Index Related to the Index class or subclasses

Comments

@toobaz
Copy link
Member

toobaz commented Aug 14, 2018

Code Sample, a copy-pastable example if possible

In [1]: import pandas as pd

In [2]: idx = pd.Index([2, 4, 6])

In [3]: idx.__inv__()
Out[3]: Int64Index([-2, -4, -6], dtype='int64')

Problem description

idx.__inv__ does what idx.__neg__ does, and not do what its name suggests (that is, operator.inv). By the way, ~idx raises TypeError anyway, and idx.__invert__ does not exist, differently from what the Python docs suggest.

Notice that instead

In [4]: s = pd.Series([2, 4, 6])

In [5]: ~s
Out[5]: 
0   -3
1   -5
2   -7
dtype: int64

In [6]: s.__invert__()
Out[6]: 
0   -3
1   -5
2   -7
dtype: int64

which is correct (and Series.__inv__ does not exist).

Expected Output

Out[3]: Int64Index([-3, -5, -7], dtype='int64')

Given how wrong, undocumented and not exposed this is, I suggest to immediately remove Index.__inv__. Then we can later decide to introduce Index.__invert__ which does the right thing, and also enable the ~ operator.

Output of pd.show_versions()

INSTALLED VERSIONS

commit: None
python: 3.5.3.final.0
python-bits: 64
OS: Linux
OS-release: 4.9.0-6-amd64
machine: x86_64
processor:
byteorder: little
LC_ALL: None
LANG: it_IT.UTF-8
LOCALE: it_IT.UTF-8

pandas: 0.24.0.dev0+460.geb0ac5437
pytest: 3.5.0
pip: 9.0.1
setuptools: 39.2.0
Cython: 0.28.4
numpy: 1.14.3
scipy: 0.19.0
pyarrow: None
xarray: None
IPython: 6.2.1
sphinx: 1.5.6
patsy: 0.5.0
dateutil: 2.7.3
pytz: 2018.4
blosc: None
bottleneck: 1.2.0dev
tables: 3.3.0
numexpr: 2.6.1
feather: 0.3.1
matplotlib: 2.2.2.post1634.dev0+ge8120cf6d
openpyxl: 2.3.0
xlrd: 1.0.0
xlwt: 1.3.0
xlsxwriter: 0.9.6
lxml: 4.1.1
bs4: 4.5.3
html5lib: 0.999999999
sqlalchemy: 1.0.15
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: 0.2.1
gcsfs: None

@toobaz toobaz added Bug Indexing Related to indexing on series/frames, not to indexes themselves Clean labels Aug 14, 2018
toobaz added a commit to toobaz/pandas that referenced this issue Aug 14, 2018
@jreback jreback added this to the 0.24.0 milestone Aug 14, 2018
@jorisvandenbossche
Copy link
Member

By the way, ~idx raises TypeError anyway

Do you know why that is? Because in principle, if __inv__() works, so should ~ (that might indicate another bug in our code)

The TypeError is raised by the Index.__inv__ implementation, but Int64Index overrides that with a working implementation.

toobaz added a commit to toobaz/pandas that referenced this issue Aug 14, 2018
@toobaz
Copy link
Member Author

toobaz commented Aug 14, 2018

Do you know why that is? Because in principle, if inv() works, so should ~ (that might indicate another bug in our code)

To the best of my understanding, and at least from Python 2.7 onwards, ~ is overridden just by using __invert__, not __inv__ (although the latter is exposed by the operators module).

@jreback jreback modified the milestones: 0.24.0, Contributions Welcome Dec 2, 2018
@toobaz toobaz added Index Related to the Index class or subclasses and removed Indexing Related to indexing on series/frames, not to indexes themselves labels Jun 29, 2019
@jbrockmendel
Copy link
Member

We define __invert__ in 5 places, __inv__ in one. Is one of these wrong?

@toobaz
Copy link
Member Author

toobaz commented Oct 18, 2020

Given that __inv__ has the wrong name (for what it does), and that the name is of an obscure method (not supported by any other pandas object, not sure it even exists in numpy, and at the same time trivial to compute as 1/idx), I think this bug should be fixed by:

(unless the first point was done already - can't test at the moment what ~idx does in master)

@toobaz
Copy link
Member Author

toobaz commented Oct 18, 2020

By the way, I'm not even sure a method which only exists as __inv__ requires a deprecation cycle...

@jbrockmendel
Copy link
Member

closed by #45006

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Clean Index Related to the Index class or subclasses
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants