BUG: positional getitem indexing with list on Series with duplicate integer index fails #26083

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.

Sign up for GitHub

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

Jump to bottom

Merged

jreback merged 8 commits into pandas-dev:master from tranctan:dup-indexing-test-case

Apr 16, 2019

+8 −0

Contributor

tranctan commented Apr 14, 2019 •

edited

Loading

Create test case for issue GH17347

closes BUG: positional getitem indexing with list on Series with duplicate integer index fails #17347
tests added / passed


          TST GH17347

c0fe2e0

Create test case for issue GH17347

pep8speaks commented Apr 14, 2019 •

edited

Loading

Hello @tranctan96! 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-04-15 17:04:32 UTC

codecov bot commented Apr 14, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@190a69e). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #26083   +/-   ##
=========================================
  Coverage          ?   91.94%           
=========================================
  Files             ?      175           
  Lines             ?    52444           
  Branches          ?        0           
=========================================
  Hits              ?    48221           
  Misses            ?     4223           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.5% <ø> (?)`
#single	`40.75% <ø> (?)`

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 190a69e...c0fe2e0. Read the comment docs.

codecov bot commented Apr 14, 2019 •

edited

Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@190a69e). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #26083   +/-   ##
=========================================
  Coverage          ?   91.95%           
=========================================
  Files             ?      175           
  Lines             ?    52412           
  Branches          ?        0           
=========================================
  Hits              ?    48195           
  Misses            ?     4217           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.51% <ø> (?)`
#single	`40.73% <ø> (?)`

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 190a69e...d25af61. Read the comment docs.


          TST GH17347

312e082

Create test case for issue GH17347

mroeschke requested changes

View reviewed changes

Member

mroeschke left a comment

Some comments.

pandas/tests/indexing/test_indexing.py Outdated

+                      # Assert the duplicate indexes
+                      for idx in dup_idxes:
+                          assert s[idx].equals(s[[idx]])

Member

mroeschke Apr 14, 2019

Use tm.assert_series_equal

pandas/tests/indexing/test_indexing.py Outdated

+                  def test_duplicate_int_indexing(self, idx):
+                      # GH 17347
+                      s = pd.Series(range(len(idx)), idx)
+                      dup_idx_filter = s.index.duplicated(keep=False)

Member

mroeschke Apr 14, 2019

What's the purpose for all this duplicated logic? You can just copy the example from the original issue.

Contributor Author

tranctan Apr 14, 2019

Hi ! Thank you for your review. This is my first time making a test case contribution, so i just figured it out that i overdid it. I'm fixing it now!

mroeschke added Indexing Testing labels

Tan Tran added 2 commits

April 15, 2019 01:25


          simply test case and switch to tm.assert

34c9f85


          simplify test case and switch to tm.assert

f5c2d54

mroeschke reviewed

View reviewed changes

pandas/tests/indexing/test_indexing.py Outdated

+                  def test_duplicate_int_indexing(self):
+                      # GH 17347
+                      s = pd.Series(range(3), index=[1, 1, 3])
+                      tm.assert_series_equal(s[1], s[[1]])

Member

mroeschke Apr 14, 2019

Can you also test loc?

And format the test like

result = ...
expected = ...
tm.assert_series_equal(result, expected)

Tan Tran added 2 commits

April 15, 2019 09:09


          refractor code and check loc case

4015bf5


          Fix checks_and_docs

f4ce36f

jreback changed the title ~~TST GH17347~~ BUG: positional getitem indexing with list on Series with duplicate integer index fails

jreback added this to the 0.25.0 milestone

jreback requested changes

View reviewed changes

pandas/tests/indexing/test_indexing.py Outdated

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

Contributor

jreback Apr 15, 2019

use parameterize like

@pytest.parameterize('case', [lambda s: s.__getitem__, lambda: s.loc])
def test_duplicate_int_indexing(self, case):
     expected = s.iloc[[1]]
     result = case(s)[[1]]

pandas/tests/indexing/test_indexing.py Outdated

+                  def test_duplicate_int_indexing(self):
+                      # GH 17347
+                      s = pd.Series(range(3), index=[1, 1, 3])
+                      expected = s[1]

Contributor

jreback Apr 15, 2019

move this after def test_dups_fancy_indexing2(self)

Tan Tran added 2 commits

April 15, 2019 23:15


          refactor code using parametrize

ff0fb7a


          fix checks_and_doc

d25af61

jreback approved these changes

View reviewed changes

jreback merged commit 4b1624e into pandas-dev:master

Contributor

jreback commented Apr 16, 2019

thanks @tranctan96

tranctan deleted the dup-indexing-test-case branch

April 18, 2019 07:08

yhaque1213 pushed a commit to yhaque1213/pandas that referenced this pull request


          BUG: positional getitem indexing with list on Series with duplicate i…

f8bc4c5

…nteger index fails (pandas-dev#26083)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Indexing Testing