From 7d1efac4ebe35faa2dee264d502c38bfc58cee7d Mon Sep 17 00:00:00 2001
From: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
Date: Wed, 16 Oct 2019 13:34:58 +0200
Subject: [PATCH] Fix remove pattern from matcher (#4454)

* raise specific error when removing a matcher rule that doesn't exist

* rephrasing

* bugfix in remove matcher + extended unit test
---
 spacy/matcher/matcher.pyx                 |  2 +-
 spacy/tests/matcher/test_matcher_logic.py | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/spacy/matcher/matcher.pyx b/spacy/matcher/matcher.pyx
index ea6a5a892..4a5f69ff4 100644
--- a/spacy/matcher/matcher.pyx
+++ b/spacy/matcher/matcher.pyx
@@ -141,7 +141,7 @@ cdef class Matcher:
         cdef int i = 0
         while i < self.patterns.size():
             pattern_key = get_ent_id(self.patterns.at(i))
-            if pattern_key == key:
+            if pattern_key == norm_key:
                 self.patterns.erase(self.patterns.begin()+i)
             else:
                 i += 1
diff --git a/spacy/tests/matcher/test_matcher_logic.py b/spacy/tests/matcher/test_matcher_logic.py
index 003383757..9469c68bb 100644
--- a/spacy/tests/matcher/test_matcher_logic.py
+++ b/spacy/tests/matcher/test_matcher_logic.py
@@ -3,6 +3,8 @@ from __future__ import unicode_literals
 
 import pytest
 import re
+
+from spacy.lang.en import English
 from spacy.matcher import Matcher
 from spacy.tokens import Doc, Span
 
@@ -145,16 +147,27 @@ def test_matcher_sets_return_correct_tokens(en_vocab):
     assert texts == ["zero", "one", "two"]
 
 
-def test_matcher_remove(en_vocab):
-    matcher = Matcher(en_vocab)
+def test_matcher_remove():
+    nlp = English()
+    matcher = Matcher(nlp.vocab)
+    text = "This is a test case."
+
     pattern = [{"ORTH": "test"}, {"OP": "?"}]
     assert len(matcher) == 0
     matcher.add("Rule", None, pattern)
     assert "Rule" in matcher
 
+    # should give two matches
+    results1 = matcher(nlp(text))
+    assert(len(results1) == 2)
+
     # removing once should work
     matcher.remove("Rule")
 
+    # should not return any maches anymore
+    results2 = matcher(nlp(text))
+    assert (len(results2) == 0)
+
     # removing again should throw an error
     with pytest.raises(ValueError):
         matcher.remove("Rule")