False positive matches on subsequent OPs in the Matcher patterns

[marina-sp]

Hello, I'm working with spacy a lot, and I especially like the spacy.matchers! But I think I found something that is not working properly (yet).

If a pattern contains a pattern element with a specified OP followed by another optional pattern element with OP=?, then all other restrictions one the first element (like ORTH or REGEX) are ignored.

How to reproduce

import spacy
import de_core_news_sm
from spacy.matcher import Matcher

nlp = de_core_news_sm.load()

text = 'Das ist ein Test.'
doc = nlp(text)
assert(len(doc) == 5)

pattern1 = [
    {'ORTH': 'Das', 'OP': '?'}, 
    {'OP': '?'},
    {'ORTH': 'Test'}
]
matcher = Matcher(nlp.vocab)
matcher.add('rule1', None, pattern1)
matches = matcher(doc) 
assert(len(matches) == 2)   # fails because of a FP match 'ist ein Test'

False positives appear on other OPs as well:

pattern2 = [
    {'ORTH': 'Das', 'OP': '+'}, 
    {'OP': '?'},
    {'ORTH': 'Test'}
]
matcher = Matcher(nlp.vocab)
matcher.add('rule1', None, pattern2)
matches = matcher(doc) 
assert(len(matches) == 0)   # fails because of a FP match 'Das ist ein Test'

pattern3 = [
    {'ORTH': 'Das', 'OP': '*'}, 
    {'OP': '?'},
    {'ORTH': 'Test'}
]
matcher = Matcher(nlp.vocab)
matcher.add('rule1', None, pattern3)
matches = matcher(doc) 
assert(len(matches) == 2)   # fails because of a 2 FP matches 'Das ist ein Test' and 'ist ein Test'

The ? operator seems to respect ORTH restrictions normally, when there is only a single operator:

pattern4 = [
    {'ORTH': 'Das', 'OP': '?'}, 
    {'ORTH': 'Test'}
]
matcher = Matcher(nlp.vocab)
matcher.add('rule1', None, pattern4)
matches = matcher(doc) 
assert(len(matches) == 1) 

My Environment

• spaCy version: 2.1.4
• Platform: Windows-10-10.0.17763-SP0
• Python version: 3.7.3

I hope this is helpful and you will be able to look into it. Thank you in advance!

[honnibal]

Thanks for the report. I think this is the same bug as #3839. Merging this with that issue.

[paulrinckens]

It seems as the fix that fixed #3839, did not fix this issue.
The above snippet still fails with v2.1.6

[svlandeg]

@paulrinckens : you're right, the matcher still produces one false positive match by misinterpreting the first token of the pattern. Reopening and wrote a unit test with @marina-sp's example.

[ines]

Merging this with #4154!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.