Correct type inference for UInt64Index during access #29420

oguzhanogreden · 2019-11-05T21:28:23Z

closes BUG: Series.loc[list] with uint64 keys raises KeyError (converted to floats) #28023 and closes Series.loc[list] with uint64 keys returns a dataframe with Float64Index #28279
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

…fault for libs.lib.infer_dtype()

alimcmaster1 · 2019-11-05T22:09:48Z

pandas/tests/indexes/test_numeric.py

+
+def test_uint64_keys_in_list():
+    # https://github.com/pandas-dev/pandas/issues/28023
+    bug = pd.Series(


Could both these test functions be combined into 1 func? Seem very similar

They are indeed similar. I'll wait for a few more comments to decide on this.

yea pls do we prefer parametized tests as much as possible

Revisiting the issues, I realized one of the tests would be redundant and removed that.

Revisiting the issues, I realized one of the tests would be redundant and removed that.

How come? - it seems that test case was exactly covering issue #28023

I would just use https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.testing.assert_index_equal.html

Hope that helps

#28023 reports a KeyError. If https://github.com/pandas-dev/pandas/pull/29420/files#diff-7c97f2c17a9193409668a6f698905c73R1206 holds, KeyError won't be raised, I think

About the test function... Now that I'm rooting for a single test, I'll leave it as isinstance since I think the intention of test is clearer this way.

pandas/tests/indexes/test_numeric.py

alimcmaster1 · 2019-11-05T22:14:59Z

Thanks for the PR!

alimcmaster1 · 2019-11-06T23:53:13Z

Test failures related to #29432

pandas/core/indexes/numeric.py

jschendel · 2019-11-08T21:37:08Z

pandas/tests/indexes/test_numeric.py

+
+    assert isinstance(
+        bug.loc[[7606741985629028552, 17876870360202815256]].index, UInt64Index
+    )


Can you explicitly construct the expected index and use tm.assert_index_equal to verify they're the same:

result = s.loc[[7606741985629028552, 17876870360202815256]].index expected = UInt64Index([7606741985629028552, 17876870360202815256]) tm.assert_index_equal(result, expected)

I'd rather not do a simple isinstance check here because it doesn't guard against potential precision loss with the values in the index, e.g. if someone makes a change where there's an intermediate coercion to Float64Index:

In [2]: idx = pd.UInt64Index([2**53, 2**53 + 1]) In [3]: idx Out[3]: UInt64Index([9007199254740992, 9007199254740993], dtype='uint64') In [4]: pd.UInt64Index(pd.Float64Index(idx)) Out[4]: UInt64Index([9007199254740992, 9007199254740992], dtype='uint64')

pep8speaks · 2019-11-10T11:48:35Z

Hello @oguzhanogreden! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-11-23 15:22:46 UTC

oguzhanogreden · 2019-11-10T11:53:23Z

This will fail due to #29526.

oguzhanogreden · 2019-11-18T21:21:42Z

Between two laptops and a rebase, git happened again. Sorry for the spam!

doc/source/whatsnew/v1.0.0.rst

pandas/tests/indexes/test_numeric.py

oguzhanogreden · 2019-11-23T15:31:23Z

@jschendel here you suggested that the What's New entry goes under Numeric. It's not clear to me why these are not under Indexing. Can you confirm that this is where the last 4 entries do belong?

jschendel · 2019-11-26T04:59:30Z

The "Indexing" section is generally for operations that have to do with selecting data with an index, e.g. loc/iloc, and the associated index methods that support this, e.g. get_loc/get_indexer. For example, see the types of things tested in pandas/tests/indexing. The "Numeric" section deals more with issues related to numeric operations, e.g. computation, precision, inference, etc.

jreback · 2019-11-27T20:47:49Z

thanks for the patch @oguzhanogreden

…ndexing-1row-df * upstream/master: (32 commits) DEPR: Series.cat.categorical (pandas-dev#29914) DEPR: infer_dtype default for skipna is now True (pandas-dev#29876) Fix broken asv (pandas-dev#29906) DEPR: Remove weekday_name (pandas-dev#29831) Fix mypy errors for pandas\tests\series\test_operators.py (pandas-dev#29826) CI: Setting path only once in GitHub Actions (pandas-dev#29867) DEPR: passing td64 data to DTA or dt64 data to TDA (pandas-dev#29794) CLN: remove unsupported sparse code from io.pytables (pandas-dev#29863) x.__class__ TO type(x) (pandas-dev#29889) DEPR: ftype, ftypes (pandas-dev#29895) REF: use named funcs instead of lambdas (pandas-dev#29841) Correct type inference for UInt64Index during access (pandas-dev#29420) CLN: follow-up to 29725 (pandas-dev#29890) CLN: trim unnecessary code in indexing tests (pandas-dev#29845) TST added test for groupby agg on mulitlevel column (pandas-dev#29772) (pandas-dev#29866) mypy fix (pandas-dev#29891) Typing annotations (pandas-dev#29850) Fix mypy error in pandas/tests.indexes.test_base.py (pandas-dev#29188) CLN: remove never-used kwargs, make kwargs explicit (pandas-dev#29873) TYP: Added typing to __eq__ functions (pandas-dev#29818) ...

oguzhanogreden added 5 commits November 5, 2019 20:53

Adds test case and fix suggestion

606b71d

Uses skipna=False default for libs.lib.infer_dtype()

7b0eef4

Reverts last commit due to deprecation warning - Uses skipna=False de…

2b537c6

…fault for libs.lib.infer_dtype()

Add test case for #28279

82cdc80

Passes black and flake8

d1818dd

alimcmaster1 requested changes Nov 5, 2019

View reviewed changes

alimcmaster1 reviewed Nov 5, 2019

View reviewed changes

pandas/tests/indexes/test_numeric.py Outdated Show resolved Hide resolved

alimcmaster1 added the Indexing Related to indexing on series/frames, not to indexes themselves label Nov 5, 2019

Removes unnecessary test

3e370b2

oguzhanogreden changed the title ~~Indexing inference pr~~ Correct type inference for UInt64Index during access Nov 6, 2019

Rename test

1722116

jschendel reviewed Nov 8, 2019

View reviewed changes

oguzhanogreden mentioned this pull request Nov 10, 2019

Non-unique integers coerced to float during UInt64Index creation with explicit #29526

Closed

Clarifies logic & tests explicitly

500e73c

Formatter...

813ed20

oguzhanogreden mentioned this pull request Nov 10, 2019

Makes NumericIndex constructor dtype aware #29529

Merged

6 tasks

oguzhanogreden added 3 commits November 18, 2019 22:30

Replay changes from #29529 & replay whatsnew in master

78db1e8

Fixed test and added whatsnew entry

a5d20d9

Merge branch 'master' into indexing-inference-pr

03b91b1

jreback requested changes Nov 20, 2019

View reviewed changes

doc/source/whatsnew/v1.0.0.rst Outdated Show resolved Hide resolved

jreback added this to the 1.0 milestone Nov 20, 2019

jreback requested changes Nov 20, 2019

View reviewed changes

doc/source/whatsnew/v1.0.0.rst Outdated Show resolved Hide resolved

pandas/tests/indexes/test_numeric.py Outdated Show resolved Hide resolved

Addressed review

aac01ea

jreback approved these changes Nov 27, 2019

View reviewed changes

jreback merged commit 70f1c28 into pandas-dev:master Nov 27, 2019

oguzhanogreden deleted the indexing-inference-pr branch November 28, 2019 12:21

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

Correct type inference for UInt64Index during access (pandas-dev#29420)

75faa5c

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

Correct type inference for UInt64Index during access (pandas-dev#29420)

0031198

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correct type inference for UInt64Index during access #29420

Correct type inference for UInt64Index during access #29420

oguzhanogreden commented Nov 5, 2019 •

edited

Loading

alimcmaster1 Nov 5, 2019

oguzhanogreden Nov 6, 2019

jreback Nov 6, 2019

oguzhanogreden Nov 6, 2019

alimcmaster1 Nov 6, 2019

alimcmaster1 Nov 6, 2019 •

edited

Loading

oguzhanogreden Nov 7, 2019 •

edited

Loading

oguzhanogreden Nov 7, 2019

alimcmaster1 commented Nov 5, 2019

alimcmaster1 commented Nov 6, 2019 •

edited

Loading

jschendel Nov 8, 2019

pep8speaks commented Nov 10, 2019 •

edited

Loading

oguzhanogreden commented Nov 10, 2019

oguzhanogreden commented Nov 18, 2019

oguzhanogreden commented Nov 23, 2019

jschendel commented Nov 26, 2019 •

edited

Loading

jreback commented Nov 27, 2019

Correct type inference for UInt64Index during access #29420

Correct type inference for UInt64Index during access #29420

Conversation

oguzhanogreden commented Nov 5, 2019 • edited Loading

alimcmaster1 Nov 5, 2019

Choose a reason for hiding this comment

oguzhanogreden Nov 6, 2019

Choose a reason for hiding this comment

jreback Nov 6, 2019

Choose a reason for hiding this comment

oguzhanogreden Nov 6, 2019

Choose a reason for hiding this comment

alimcmaster1 Nov 6, 2019

Choose a reason for hiding this comment

alimcmaster1 Nov 6, 2019 • edited Loading

Choose a reason for hiding this comment

oguzhanogreden Nov 7, 2019 • edited Loading

Choose a reason for hiding this comment

oguzhanogreden Nov 7, 2019

Choose a reason for hiding this comment

alimcmaster1 commented Nov 5, 2019

alimcmaster1 commented Nov 6, 2019 • edited Loading

jschendel Nov 8, 2019

Choose a reason for hiding this comment

pep8speaks commented Nov 10, 2019 • edited Loading

Comment last updated at 2019-11-23 15:22:46 UTC

oguzhanogreden commented Nov 10, 2019

oguzhanogreden commented Nov 18, 2019

oguzhanogreden commented Nov 23, 2019

jschendel commented Nov 26, 2019 • edited Loading

jreback commented Nov 27, 2019

oguzhanogreden commented Nov 5, 2019 •

edited

Loading

alimcmaster1 Nov 6, 2019 •

edited

Loading

oguzhanogreden Nov 7, 2019 •

edited

Loading

alimcmaster1 commented Nov 6, 2019 •

edited

Loading

pep8speaks commented Nov 10, 2019 •

edited

Loading

jschendel commented Nov 26, 2019 •

edited

Loading