Skip to content

Conversation

@jaredsnyder
Copy link
Contributor

@jaredsnyder jaredsnyder commented May 22, 2017

Switched out
values = np.array(list(values), dtype='object')
for
values = lib.list_to_object_array(list(values))
in the isin() method found in core/algorithms.py

Added test for comparing to a list of tuples

…lues = lib.list_to_object_array(list(values))" in the isin() method found in core/algorithms.py

Added test for comparing to a list of tuples
@jorisvandenbossche jorisvandenbossche added this to the 0.20.2 milestone May 22, 2017
@jorisvandenbossche jorisvandenbossche changed the title Fix for issue #16394 BUG: fix isin with Series of tuples values (#16394) May 22, 2017
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test looks good, let's wait what Travis says.

Can you add a small entry about this fix in the whatsnew file? This can be in doc/source/whatsnew/v0.20.2.txt

df['C'] = list(zip(df['A'], df['B']))
result = df['C'].isin([(1, 'a')])
tm.assert_series_equal(result,
Series([True,False,False],name="C"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for PEP8: the indentation does not fully match with the line before (one space more needed), and a space after the comma is needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that, fixed

tm.assert_frame_equal(result, expected)

def test_isin_tuples(self):
df = pd.DataFrame({'A': [1, 2, 3], 'B': ['a', 'b', 'f']})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the original issue number as a comment on the first line ?
just like # GH16394

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@jaredsnyder
Copy link
Contributor Author

Added the following to the whatsnew file under Bugs - Resizing:

  • Bug in isin in core/algorithms.py with comparing lists of tuples

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, minor doc comment.


- Bug in ``DataFrame.stack`` with unsorted levels in MultiIndex columns (:issue:`16323`)

- Bug in ``isin`` in core/algorithms.py with comparing lists of tuples
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Series.isin(..) with a list of tuples. add the issue number here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, think I got the format correct with the latest commit

@jreback
Copy link
Contributor

jreback commented May 22, 2017

@jaredsnyder I think you just a couple of lint issues. https://travis-ci.org/pandas-dev/pandas/jobs/234970029

@jreback jreback added the Reshaping Concat, Merge/Join, Stack/Unstack, Explode label May 22, 2017
@codecov
Copy link

codecov bot commented May 22, 2017

Codecov Report

Merging #16434 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #16434   +/-   ##
=======================================
  Coverage   90.42%   90.42%           
=======================================
  Files         161      161           
  Lines       51023    51023           
=======================================
  Hits        46138    46138           
  Misses       4885     4885
Flag Coverage Δ
#multiple 88.26% <100%> (ø) ⬆️
#single 40.17% <100%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/algorithms.py 94.41% <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 49ec31b...4dc6bc5. Read the comment docs.

@codecov
Copy link

codecov bot commented May 22, 2017

Codecov Report

Merging #16434 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16434      +/-   ##
==========================================
- Coverage   90.42%   90.42%   -0.01%     
==========================================
  Files         161      161              
  Lines       51023    51023              
==========================================
- Hits        46138    46137       -1     
- Misses       4885     4886       +1
Flag Coverage Δ
#multiple 88.26% <100%> (-0.01%) ⬇️
#single 40.17% <100%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/algorithms.py 94.41% <100%> (ø) ⬆️
pandas/core/common.py 91.05% <0%> (-0.34%) ⬇️

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 49ec31b...ce622e6. Read the comment docs.


- Bug in ``DataFrame.stack`` with unsorted levels in MultiIndex columns (:issue:`16323`)

- Bug in ``Series.isin(..)`` in core/algorithms.py with a list of tuples (:issue:`16394`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug in Series.isin(...) with a list of tuples. No need to mention internal things.

@jreback
Copy link
Contributor

jreback commented May 23, 2017

minor comment. ping on green.

@jaredsnyder
Copy link
Contributor Author

@jreback addressed your comment

@jorisvandenbossche jorisvandenbossche merged commit e053ee3 into pandas-dev:master May 23, 2017
@jorisvandenbossche
Copy link
Member

@jaredsnyder Thanks!

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request May 29, 2017
…-dev#16434)

* Swiched out "values = np.array(list(values), dtype='object')" for "values = lib.list_to_object_array(list(values))" in the isin() method found in core/algorithms.py
Added test for comparing to a list of tuples

(cherry picked from commit e053ee3)
TomAugspurger pushed a commit that referenced this pull request May 30, 2017
* Swiched out "values = np.array(list(values), dtype='object')" for "values = lib.list_to_object_array(list(values))" in the isin() method found in core/algorithms.py
Added test for comparing to a list of tuples

(cherry picked from commit e053ee3)
stangirala pushed a commit to stangirala/pandas that referenced this pull request Jun 11, 2017
…-dev#16434)

* Swiched out "values = np.array(list(values), dtype='object')" for "values = lib.list_to_object_array(list(values))" in the isin() method found in core/algorithms.py
Added test for comparing to a list of tuples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: ValueError with Series.isin and tuples

4 participants