BUG #19860 Corrected use of mixed indexes with .at #22436

mariuspot · 2018-08-21T06:54:14Z

.at incorrectly disallowed the use of integer indexes when a mixed index was used

s = Series([1,2,3,4,5],index=['a','b','c',1,2])
s.at['a'] # returns 1
s.at[1]   # returns 4
s.at[4]   # raises KeyError, doesn't do fallback indexing

Made sure fallback indexing doesn't happen on mixed indexes

closes BUG?: .at not working on object indexes containing some integers #19860
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

.at incorrectly disallowed the use of integer indexes when a mixed index was used disabled fallback indexing when a mixed index is used

codecov · 2018-08-21T09:39:55Z

Codecov Report

❗ No coverage uploaded for pull request base (master@fa47b8d). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #22436   +/-   ##
=========================================
  Coverage          ?   92.03%           
=========================================
  Files             ?      169           
  Lines             ?    50780           
  Branches          ?        0           
=========================================
  Hits              ?    46737           
  Misses            ?     4043           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.44% <100%> (?)`
#single	`42.22% <33.33%> (?)`

Impacted Files	Coverage Δ
pandas/core/indexing.py	`93.79% <100%> (ø)`
pandas/core/indexes/base.py	`96.45% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa47b8d...fcb1f61. Read the comment docs.

jreback

looks pretty good. can you add a whatsnew note in bugfixes indexing for 0.24

jreback · 2018-08-21T10:29:51Z

pandas/tests/indexing/test_indexing.py

@@ -666,6 +666,20 @@ def test_index_type_coercion(self):
                    idxr(s2)['0'] = 0
                    assert s2.index.is_object()

+    @pytest.mark.parametrize("index,val", [


can you add a comment with the issue number

these tests should be in pandas/tests/indexes/test_base (look for other contains tests)

test_index_not_contains and test_index_contains are in the same file a few lines above, should I move these up or create pandas/tests/indexes/test_base and move them there?

jreback · 2018-08-21T10:31:37Z

pandas/tests/indexing/test_indexing.py

@@ -710,6 +724,27 @@ def test_float_index_at_iat(self):
        for i in range(len(s)):
            assert s.iat[i] == i + 1

+    def test_mixed_index_at_iat(self):


move these to pandas/tests/indexing/test_scalar.py (see where other ones are)

add a gh comment with the issue number

can you add the test from the OP (was on dataframe)

jreback · 2018-08-21T10:34:14Z

pandas/tests/indexing/test_indexing.py

+        s = Series([1, 2, 3, 4, 5], index=['a', 'b', 'c', 1, 2])
+        for el, item in s.iteritems():
+            assert s.at[el] == item
+        for i in range(len(s)):


can you check that we have similar tests for .loc (I think so), if not can you add a loc version (inside same test is ok). maybe even just add the .loc tests and move from elsewhere.

you can parameterize these over .at / .loc

jreback · 2018-08-21T10:35:15Z

also there might be other .at issues which this closes. pls have a look if you can.

cc @toobaz

Added whatsnew to 0.24 Added issue to tests comments Moved some test Added a test from the issue

pep8speaks · 2018-08-21T12:59:30Z

Hello @mariuspot! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on August 27, 2018 at 15:41 Hours UTC

jreback · 2018-08-22T10:37:01Z

looks good. can you rebase

jreback · 2018-08-22T12:31:19Z

looks like you picked up some other commits, do this
git merge upstream/master and push again

mariuspot · 2018-08-22T13:01:25Z

git merge upstream/master didn't do anything
Not sure what I did now :/
Hope it's ok now

mariuspot · 2018-08-23T05:02:53Z

Ok, I figured out what went wrong with the merge. Should be ok now

jreback

lgtm. looks like some extraneous rebase issue. ping on green.

jreback · 2018-08-23T10:26:40Z

doc/source/whatsnew/v0.24.0.txt

@@ -659,7 +659,11 @@ Indexing
 - Fixed ``DataFrame[np.nan]`` when columns are non-unique (:issue:`21428`)
 - Bug when indexing :class:`DatetimeIndex` with nanosecond resolution dates and timezones (:issue:`11679`)
 - Bug where indexing with a Numpy array containing negative values would mutate the indexer (:issue:`21867`)
+<<<<<<< HEAD


hve a rebase issue here

jreback · 2018-08-23T10:32:40Z

lgtm ping on green.

mariuspot · 2018-08-23T18:34:16Z

Looks like travis failed with a segfault on the linker

mariuspot · 2018-08-27T04:46:22Z

@jreback The travis build had a segfault with the linker while setting up the environment. What is the process for that. Is there a way for me to rerun the build or should I make a change and do a new commit?

jreback · 2018-08-29T12:49:05Z

thanks!

…dev#22436)

BUG pandas-dev#19860 Corrected use of mixed indexes with .at

f47ba8e

.at incorrectly disallowed the use of integer indexes when a mixed index was used disabled fallback indexing when a mixed index is used

jreback requested changes Aug 21, 2018

View reviewed changes

jreback added Bug Indexing Related to indexing on series/frames, not to indexes themselves labels Aug 21, 2018

Fixed issues from review

46025b0

Added whatsnew to 0.24 Added issue to tests comments Moved some test Added a test from the issue

Fixed PEP8 issues :(

bef65fc

jreback added this to the 0.24.0 milestone Aug 22, 2018

jreback requested a review from toobaz August 22, 2018 10:37

Merge remote-tracking branch 'upstream/master' into issue-19860

9e55818

mariuspot force-pushed the issue-19860 branch from c11626d to 9e55818 Compare August 23, 2018 05:01

jreback requested changes Aug 23, 2018

View reviewed changes

Fixed rebase issue

9a50f04

jreback approved these changes Aug 23, 2018

View reviewed changes

Merge branch 'master' of github.com:pandas-dev/pandas into issue-19860

fcb1f61

jreback merged commit bf67634 into pandas-dev:master Aug 29, 2018

mariuspot deleted the issue-19860 branch September 10, 2018 05:48

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

BUG pandas-dev#19860 Corrected use of mixed indexes with .at (pandas-…

5d1af2f

…dev#22436)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG #19860 Corrected use of mixed indexes with .at #22436

BUG #19860 Corrected use of mixed indexes with .at #22436

mariuspot commented Aug 21, 2018

codecov bot commented Aug 21, 2018 •

edited

jreback left a comment

jreback Aug 21, 2018

jreback Aug 21, 2018

mariuspot Aug 21, 2018

jreback Aug 21, 2018

jreback Aug 21, 2018

jreback Aug 21, 2018

jreback Aug 21, 2018

jreback commented Aug 21, 2018

pep8speaks commented Aug 21, 2018 •

edited

jreback commented Aug 22, 2018

jreback commented Aug 22, 2018 •

edited

mariuspot commented Aug 22, 2018

mariuspot commented Aug 23, 2018

jreback left a comment

jreback Aug 23, 2018

jreback commented Aug 23, 2018

mariuspot commented Aug 23, 2018

mariuspot commented Aug 27, 2018

jreback commented Aug 29, 2018

BUG #19860 Corrected use of mixed indexes with .at #22436

BUG #19860 Corrected use of mixed indexes with .at #22436

Conversation

mariuspot commented Aug 21, 2018

codecov bot commented Aug 21, 2018 • edited

Codecov Report

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Aug 21, 2018

pep8speaks commented Aug 21, 2018 • edited

Comment last updated on August 27, 2018 at 15:41 Hours UTC

jreback commented Aug 22, 2018

jreback commented Aug 22, 2018 • edited

mariuspot commented Aug 22, 2018

mariuspot commented Aug 23, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Aug 23, 2018

mariuspot commented Aug 23, 2018

mariuspot commented Aug 27, 2018

jreback commented Aug 29, 2018

codecov bot commented Aug 21, 2018 •

edited

pep8speaks commented Aug 21, 2018 •

edited

jreback commented Aug 22, 2018 •

edited