BUG: string methods with NA

[prakhar987]

• closes Bug in Series.str.repeat with string dtype and sequence of repeats #31632
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[prakhar987]

Have added _typ to class NAType so it can be identified in is_nan() correctly. That's one way of doing it but probably can be done better. Will update the PR according to suggestions.

[jbrockmendel]

I don’t think we want is_nan to recognize NA.

[TomAugspurger]

Agreed. To fix #31632 we'll likely want to adjust how the na_map function is called.

[prakhar987]

thanks for the inputs...will update this PR or raise a new one

[prakhar987]

@TomAugspurger I think vec_binop is called instead of na_map because input is a vector. Should we implement a variant of vec_binop which can handle NA? Or a shorter fix might be checking for NA before calling bytes.__mul__ and str.__mul__

[TomAugspurger]

Either of those sound sensible.

…

On Wed, Feb 5, 2020 at 10:51 PM Prakhar Pandey ***@***.***> wrote: @TomAugspurger <https://github.com/TomAugspurger> I think vec_binop is called instead of na_map because input is a vector. Should we implement a variant of vec_binop which can handle NA? Or a shorter fix might be checking for NA before calling bytes.__mul__ and str.__mul__ — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#31684?email_source=notifications&email_token=AAKAOIS2IMDWKD53WV5ZOC3RBOJNBA5CNFSM4KQFW7K2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK55RWQ#issuecomment-582736090>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKAOIVMIKLXCXMXQPNSXY3RBOJNBANCNFSM4KQFW7KQ> .

[jreback]

do we need the same change for scalar_rep? (and a test that hits it)?

[prakhar987]

If sequence passed to repeat has None it is handled...have added a test for it

[jbrockmendel]

instead of isinstance, just use if x is NA

[prakhar987]

Thanks for the suggestion. Have updated.

[jreback]

can you add pd.NA in tests for the other string methods, this might be a more systematic issue.

[prakhar987]

@jreback i think other string methods will be fine because most of them use _na_map which handles pd.NA. This specific issue was due to str_repeat using vec_binop for vectored inputs which didn't handle pd.NA. Let me know if you want the tests anyway. Will add tests for NA

[prakhar987]

@jreback should i put the pd.NA tests in separate function or just add to functions that are already there?

[TomAugspurger]

can you add pd.NA in tests for the other string methods, this might be a more systematic issue.

@jreback this was the only case method using vec_binop. The rest are using _na_map which should handle NA correctly. So I think we're OK.

Thanks @prakhar987!

[TomAugspurger]

@meeseeksdev backport to 1.0.x

[lumberbot-app]

Something went wrong ... Please have a look at my logs.