Redefine match

[danielballan]

closes #5075

How does this look? If the docstring and the tests reflect our consensus, I'll take a stab at the docs. This is the gist of it:

Default behavior is unchanged, but issues a warning.

In [1]: Series(['aa', 'bb']).str.match('(a)(a)')
pandas/core/strings.py:333: UserWarning: This usage of match will be removed in
            an upcoming version of pandas. Consider using extract instead.
  UserWarning)
Out[1]: 
0    (a, a)
1        []
dtype: object

New, more useful behavior is available through as_indexer.

In [2]: Series(['aa', 'bb']).str.match('(a)(a)', as_indexer=True)
Out[2]: 
0     True
1    False
dtype: bool

P.S. There's a stray commit in here. Not sure why 88716e4 got lumped in....

[jtratner]

Today I realized there's a contains method too :-/ not sure what we want to do.

[danielballan]

Ha! Well I'm glad this didn't take too long. Hmm.

[jtratner]

I just found it today - I had no idea. I guess we should just have match be a synonym with warning for extract and then deprecate? Or just let match keep doing its thing and emit warnings til we remove it?

[jtratner]

You might see if your implementation is better than contains...

That said, if you want to fix things like this in the future, easiest thing to do is diff your final commit against master, save the diff just on the files you care about, checkout master, then use git apply with the diff and then commit it again. That or copy/paste :)

[jtratner]

(I mean fix git issues.)

[jreback]

@danielballan this needs to be rebased to eliminate the first commit

[danielballan]

@jreback It's unclear what we want to do with this now. I'm in favor of @jtratner's suggestion above: make match a synonym for extract, and scrap it in 0.14.

[jreback]

@danielballan yes...I thought that was fine for 0.13....I haven't looked at this close...is that the intention here?

[danielballan]

No, this made match return a boolean indexer, which we now realize is already covered by str.contains. Cannot fix today, but probably tomorrow.

[jtratner]

I'm still mixed on the naming here. What if we make match use re.match whereas contains can use re.search?

I'm still not sold on adding extract as a name and deprecating match. Though it might be surprising to have match return different ndim objects depending on number of groups, there's something nice about having one function work intelligently based on number of match groups.

[jreback]

how's this coming along?

[danielballan]

We need a final decision on the naming.

I have come around to preferring @jtratner's suggestion, where str.match and str.contains return boolean indexers based on re.match and re.search respectively. That leaves str.extract intact, effectively doing what str.match used to do, but in a more useful structure and (arguably) with more descriptive name.

Examples from current PR branch "redefine-match":

In [12]: Series(['abc', '123']).str.contains('bc') # uses re.search
Out[12]: 
0     True
1    False
dtype: bool

In [13]: Series(['abc', '123']).str.match('bc', as_indexer=True) # uses re.match
Out[13]: 
0    False
1    False
dtype: bool

In [14]: Series(['abc', '123']).str.match('bc') # again, re.match
UserWarning: This usage of match will be removed in
        an upcoming version of pandas. Consider using extract instead.
Out[14]: 
0    []
1    []
dtype: object

In [15]: Series(['abc', '123']).str.extract('bc') # uses re.match
ValueError: This pattern contains no groups to capture.

In [16]: Series(['abc', '123']).str.extract('(bc)')
Out[16]: 
0   NaN
1   NaN
dtype: float64

In [17]: Series(['abc', '123']).str.extract('.*(bc)')
Out[17]: 
0     bc
1    NaN
dtype: object

If all this looks good, all we need are docs.

[jtratner]

Your suggested api looks good, but I'd prefer extract use re.search (if
someone really cared to get match behavior, simple to do in the regex
itself). match should not warn if you don't give it match groups, because
the previous behavior (returning empty tuples and lists) was basically
useless, so that usage is unambiguous.

I'd prefer the match warning to say 'In future versions of pandas, match
will change to always return a bool indexer'

Similarly, if as_indexer is True and the regex has match groups (or using
str.contains and it has match groups), would be nice to warn that you can
use str.extract to actually get the match groups back.

[danielballan]

Changes made. If we all like them, I'll write docs tomorrow. Let me know.

[jreback]

fyi....seems you have an odd commit in there....can you rebase off of current master?

[jreback]

can you rebase against master?

[jreback]

@jtratner ?

[danielballan]

Sorry, nonstop work at my day job, pushing a paper out. Will do today when I get to my work machine.

[danielballan]

Fixed. Thanks for the git guidance (awhile ago) @jtratner. Ready for docs? Are we happy with this?

[jreback]

I like this.

So to summarize we are effectively deprecating str.match in favor of extract, not touching contains, which does what?

[danielballan]

0.12:
match -- extracts matches in annoying, un-useful way
contains -- returns boolean indexer based on re.search

0.13:
match -- warn on annoying/useless behavior; new as_indexer option returns boolean indexer based on re.match
extract -- effectively replaces match, returns useful structure (uses on re.search)
contains -- returns boolean indexer based on re.search

0.14:
match -- returns boolean indexer based on re.match
extract -- effectively replaces match, returns useful structure (uses on re.search)
contains -- returns boolean indexer based on re.search

[jreback]

I would say go ahead with the docs (put that in a separate commit).... for v0.13.0 and in the string section

[jreback]

@danielballan I would possibly make some sub-sections under the string methods, e.g. maybe for the methods that you are actually explaining (e.g. match/extract/split.....).

[jtratner]

I'm very happy with this (and I appreciate your flexibility!)- can you confirm one thing for me:

match -- warn on annoying/useless behavior; new as_indexer option returns boolean indexer based on re.match

So this means when match gets something with zero match groups, it returns a boolean indexer? That way for the majority of use cases match will just work. (i.e., no need to even put examples in the docs with match groups) and then we'd warn when there were match groups but preserve existing.

Thanks for putting this together!

[danielballan]

I hadn't thought of that. Currently, match only returns a boolean indexer is you set as_indexer=True. What you are suggesting is better. I will make the change.

Sorry for the delay on my end. Not losing patience, just short on free time this week.

[jreback]

@danielballan how's this coming?

[danielballan]

Starting the docs, I came across this:

Methods like contains, startswith, and endswith takes an extra
na arguement so missing values can be considered True or False.

The new version of match should do that as well. The only different between contains and the new match should be re.search vs. re.match.

[jtratner]

that's fine, fine with me if you incorporate that.

[jtratner]

Just ping me when you think this is all ready to go. Also, if you want to
rebase on master while you're working on this, would be helpful.

[jtratner]

I'd really like this to be in 0.13 rc, because it will wrap up str methods
nicely - would you be able to get this out today? No pressure tho

[danielballan]

Ready.

Now the APIs and underlying codes for match and contains are happily symmetric, aside from the warning apparatus and the as_indexer stuff that we will remove in 0.14. Docs should be good to go. I haven't complied them (I'm not set up for that yet) but I didn't touch any of the ipython code in these edits.

[jtratner]

As long as it passes the tests, I'm fine with this.

I probably would want to edit the docs somewhat to just show what you can do in 0.13 and not bother mentioning all the weird deprecated things you can do (esp since I think you can always go back and look at docs for older versions), but I'm fine with doing it later and getting this functionality merged for now :)

[danielballan]

I waffled on how much to explain, and probably erred saying too much. I'll heading back over to see if I can fix filter in time for the RC. We can come back to this.

[jtratner]

Yeah, exactly, docs are easy to pare down later - just commenting that we should do it at some point. Frankly, the entire docs section on string matching could be tightened up a bit.

[jtratner]

future reference - you can use assert_produces_warning() here as well, so you could get down to:

with tm.assert_produces_warning():
    result = values.str.match('.*(BAD[_]+).*(BAD)')

[jtratner]

Thanks!

[hayd]

One workaround/trick is to do (where regex is foo|bar):

s.str.contains('^(foo|bar)$')