FIX: add additional condition to check invalid type(#28277)

[boomsquared]

• closes series.str.cat(series.str) is concatenating only the largest string #28277
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[pep8speaks]

Hello @boomsquared! 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-09-06 19:31:31 UTC

[WillAyd]

Thanks for the PR! Can you add a test for the behavior you are trying to solve? Tests are typically the most critical part of any PR

[Mapichit]

@WillAyd Thanks for the advice, will certainly do.

[WillAyd]

Thanks again for the PR! Great investigation into root cause. I'm not sure about the proposed fixed just yet but cc @TomAugspurger if he has thoughts as well

Can you also add a whatsnew note for v1.0.0?

[WillAyd]

Not sure about this. I think the real issue is that is_list_like returns True for s.str when it shouldn't, though I do see that is also technically Iterable so maybe this should be allowed and just fixed

[TomAugspurger]

I'm not sold on the value of Series.str.__iter__, but #3638 and #3645 are interesting history. I'm not sure about the value of Series.str.__iter__, especially when it causes surprising behavior like this.

[WillAyd]

Hmm that's very surprising. Seems like it yields "columnar character by character" access across all elements?

Maybe we should just deprecate that behavior then? Having a hard time seeing where that is useful as an iterator

[boomsquared]

@WillAyd The StringMethods.get calls str_get which str_get "Extract element from lists, tuples, or strings in each element in the Series/Index". Should I fix the StringMethods.get by not calling str_get? I'm not sure how changing this affects the rest of the code.

[WillAyd]

I think the best approach would be to deprecate the __iter__ method of Series.str as noted above and remove in the future - is that something you'd be interested in trying?

[boomsquared]

@WillAyd Is there something more to depreciating the function than just removing it from the code?

[WillAyd]

Yea we would want to throw a warning for the time being and add some doc notes / sphinx directives for the deprecation. There's a Deprecate label where you can see past PRs, but citing a specific example I think #26744 is one PR that's relatively clean to emulate