8000 BUG: Fix series rename called with str altering name rather index (GH17407) by huashuai · Pull Request #17654 · pandas-dev/pandas · GitHub
[go: up one dir, main page]
Skip to content

BUG: Fix series rename called with str altering name rather index (GH17407) #17654

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

Merged
merged 8 commits into from
Sep 30, 2017
Merged

BUG: Fix series rename called with str altering name rather index (GH17407) #17654

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3e3e925
BUG: Fix series rename called with str altering the name. GH17407
huashuai Sep 24, 2017
99300dd
add whatsnew for the fix for #17407
huashuai Sep 24, 2017
b9ee088
Fix typo in whatsnew
huashuai Sep 24, 2017
4996e76
remove whitespace
huashuai Sep 24, 2017
41436c2
Update code after @jreback's comments
huashuai Sep 24, 2017
f97c9bc
Change `or` to `and` for checking iterable
huashuai Sep 25, 2017
45d30d4
Only check against Iterable in is_list_like and add test for `str`
huashuai Sep 26, 2017
a1f9565
Update v0.21.0.txt
TomAugspurger Sep 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Change or to and for checking iterable
  • Loading branch information
@huashuai
huashuai committed Sep 29, 2017
commit f97c9bc2d44f7aa58314fe67f1fa074af3032473
3 changes: 2 additions & 1 deletion pandas/core/dtypes/inference.py
View file
Open in desktop 8000
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,10 @@ def is_list_like(obj):
False
"""

return ((hasattr(obj, '__iter__') or isinstance(obj, Iterable)) and
return (hasattr(obj, '__iter__') and isinstance(obj, Iterable) and
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be an or

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see your point, but that wont fix the issue. What about only check isinstance(obj, Iterable)? I'm asking this because I see another function in inference.py called _iterable_not_string, it checks against collections.Iterable rather check for __iter__ attribute.

Copy link
Contributor

Choose a reason for hiding this comment

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

you can try but this is used pretty much everywhere and you will break things. Technically a type satisfies this condition I think, but we DO want to exlude it. you could also add it to the not isinstance(obj, (string_types, type)). might work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've checked, type actually doesn't satisfy the condition. Also all the tests have passed after the change.

not isinstance(obj, string_and_binary_types))
Copy link
Contributor
@jreback jreback Sep 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

do this with an or between iter and collections....

return (hasattr(obj, '__iter__') or isinstance(obj, Iterable) and not isinstance(obj, string....)

Copy link
Contributor Author
@huashuai huashuai Sep 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

using or doesn't work, as hasattr(str, '__iter__') returns True and that caused the bug. I changed it to use and


Copy link
Contributor

Choose a reason for hiding this comment

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

pls run a sub-set of the perf tests (IOW something like this)

asv continuous master thisbranchname --bench indexing
asv continuous master thisbranchname --bench timeseries

and report any significant diffs.

This touches code from almost everywhere indirectly so want to make sure no regressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. See comments below for what the performance tests reported.


def is_nested_list_like(obj):
"""
Check if the object is list-like, and that all of its elements
Expand Down
0