Repository: cisco/mindmeld (language: python)

Issue:
Query cache does not update with change in Tokenizers config
With the work for Multilingual Tokenizers finally merged (PR #328), users will now have an option to set different tokenizer and normalizers for the text preparation pipeline. However, the current Mindmeld query cache implementation does not update the cache with changes in text preparation config. The Mindmeld query cache currently only updates with changes in the training data.


Discussion / hints:

Fix patch:
diff --git a/.gitignore b/.gitignore
index 6b855efe6..9376c11b8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -70,3 +70,6 @@ mindmeld/resources/duckling-*
 
 # MindMeld Generated Data
 mindmeld/data/
+
+# Pytest data
+tests/converter/mm_rasa_converted_project/*
diff --git a/mindmeld/core.py b/mindmeld/core.py
index 75ebbbdd7..4e22cfe90 100644
--- a/mindmeld/core.py
+++ b/mindmeld/core.py
@@ -492,10 +492,6 @@ class ProcessedQuery:
         confidence (dict): A dictionary of the class probas for the domain and intent classifier
     """
 
-    # version the cached data.  Bump this if any changes are made
-    # to the to_cache() and from_cache() functions for the core classes.
-    version = 4
-
     def __init__(
         self,
         query,
diff --git a/mindmeld/query_cache.py b/mindmeld/query_cache.py
index 4a4016383..7b41d4db2 100644
--- a/mindmeld/query_cache.py
+++ b/mindmeld/query_cache.py
@@ -41,8 +41,9 @@ def copy_db(source, dest):
             cursor.execute(statement)
         dest.commit()
 
-    def __init__(self, app_path):
+    def __init__(self, app_path, schema_version_hash):
         # make generated directory if necessary
+        self.schema_version_hash = schema_version_hash
         gen_folder = GEN_FOLDER.format(app_path=app_path)
         if not os.path.isdir(gen_folder):
             os.makedirs(gen_folder)
@@ -68,11 +69,11 @@ def __init__(self, app_path):
         # Create table to store the data version
         cursor.execute("""
         CREATE TABLE IF NOT EXISTS version
-        (version_number INTEGER PRIMARY KEY);
+        (schema_version_hash TEXT PRIMARY KEY);
         """)
         cursor.execute("""
         INSERT OR IGNORE INTO version values (?);
-        """, (ProcessedQuery.version,))
+        """, (self.schema_version_hash,))
         self.disk_connection.commit()
 
         in_memory = bool(strtobool(os.environ.get("MM_QUERY_CACHE_IN_MEMORY", "1").lower()))
@@ -116,14 +117,11 @@ def compatible_version(self):
 
         cursor = self.disk_connection.cursor()
         try:
-            cursor.execute("""
-            SELECT version_number FROM version WHERE version_number=(?);
-            """, (ProcessedQuery.version,))
-            if len(cursor.fetchall()) == 0:
-                # version does not match
-                return False
-            return True
-        except Exception:  # pylint: disable=broad-except
+            row = cursor.execute("""
+            SELECT COUNT(schema_version_hash) FROM version WHERE schema_version_hash=(?);
+            """, (self.schema_version_hash,)).fetchone()
+            return row[0] > 0
+        except sqlite3.Error:  # pylint: disable=broad-except
             return False
 
     @staticmethod
@@ -209,7 +207,7 @@ def get(self, row_id):
         is called. Exceptions may be thrown in the case of database corruption.
 
         The method caches the previously retrieved element because it is common
-        for a set of iterators (examples and lables) to retrieve the same row_id
+        for a set of iterators (examples and labels) to retrieve the same row_id
         in sequence.  The cache prevents extra db lookups in this case.
 
         Args:
diff --git a/mindmeld/resource_loader.py b/mindmeld/resource_loader.py
index bc0d5b170..6bc8e8828 100644
--- a/mindmeld/resource_loader.py
+++ b/mindmeld/resource_loader.py
@@ -46,6 +46,7 @@ class ProcessedQueryList:
     ProcessedQueryList provides a memory efficient disk backed list representation
     for a list of queries.
     """
+
     def __init__(self, cache=None, elements=None):
         self._cache = cache
         self.elements = elements or []
@@ -177,6 +178,7 @@ class ListIterator(Iterator):
         of arbitrary lists of data and presenting them as a
         ProcessedQueryList.Iterator to functions that require them.
         """
+
         def __init__(self, elements):
             self.source = None
             self.elements = elements
@@ -191,6 +193,7 @@ class MemoryCache:
         A class to provide cache functionality for in-memory
         lists of ProcessedQuery objects
         """
+
         def __init__(self, queries):
             self.queries = queries
 
@@ -266,7 +269,9 @@ def query_cache(self):
         Lazy load the query cache since it's not required for inference.
         """
         if not self._query_cache:
-            self._query_cache = QueryCache(app_path=self.app_path)
+            text_prep_hash = self.query_factory.text_preparation_pipeline.get_hashid()
+            self._query_cache = QueryCache(app_path=self.app_path,
+                                           schema_version_hash=text_prep_hash)
         return self._query_cache
 
     @property
@@ -727,6 +732,7 @@ class WordFreqBuilder:
         """
         Compiles unigram frequency dictionary of normalized query tokens
         """
+
         def __init__(self, enable_stemming=False):
             self.enable_stemming = enable_stemming
             self.tokens = []
@@ -750,6 +756,7 @@ class CharNgramFreqBuilder:
         """
         Compiles n-gram character frequency dictionary of normalized query tokens
         """
+
         def __init__(self, lengths, thresholds):
             self.lengths = lengths
             self.thresholds = list(thresholds) + [1] * (len(lengths) - len(thresholds))
@@ -766,9 +773,9 @@ def add(self, query):
                     )
                 query_text = re.sub(r"\d", "0", query.normalized_text)
                 character_tokens = [
-                    query_text[i : i + length]
+                    query_text[i: i + length]
                     for i in range(len(query_text))
-                    if len(query_text[i : i + length]) == length
+                    if len(query_text[i: i + length]) == length
                 ]
                 self.char_freq_dict.update(character_tokens)
 
@@ -779,6 +786,7 @@ class WordNgramFreqBuilder:
         """
         Compiles n-gram frequency dictionary of normalized query tokens
         """
+
         def __init__(self, lengths, thresholds, enable_stemming=False):
             self.lengths = lengths
             self.thresholds = list(thresholds) + [1] * (len(lengths) - len(thresholds))
@@ -806,10 +814,10 @@ def add(self, query):
                                      [OUT_OF_BOUNDS_TOKEN]
 
                 for i in range(len(normalized_tokens)):
-                    ngram_query = " ".join(normalized_tokens[i : i + length])
+                    ngram_query = " ".join(normalized_tokens[i: i + length])
                     ngram_tokens.append(ngram_query)
                     if self.enable_stemming:
-                        stemmed_ngram_query = " ".join(stemmed_tokens[i : i + length])
+                        stemmed_ngram_query = " ".join(stemmed_tokens[i: i + length])
                         if stemmed_ngram_query != ngram_query:
                             ngram_tokens.append(stemmed_ngram_query)
                 self.word_freq_dict.update(ngram_tokens)
@@ -821,6 +829,7 @@ class QueryFreqBuilder:
         """
         Compiles frequency dictionary of normalized and stemmed query strings
         """
+
         def __init__(self, enable_stemming=False):
             self.enable_stemming = enable_stemming
             self.query_dict = Counter()
diff --git a/mindmeld/text_preparation/normalizers.py b/mindmeld/text_preparation/normalizers.py
index 02beb6f64..422374a33 100644
--- a/mindmeld/text_preparation/normalizers.py
+++ b/mindmeld/text_preparation/normalizers.py
@@ -22,7 +22,6 @@
 
 logger = logging.getLogger(__name__)
 
-
 ASCII_CUTOFF = ord("\u0080")
 
 
@@ -42,6 +41,18 @@ def normalize(self, text):
         """
         raise NotImplementedError("Subclasses must implement this method")
 
+    def tojson(self):
+        """
+        Method defined to obtain recursive JSON representation of a TextPreparationPipeline.
+
+        Args:
+            None.
+
+        Returns:
+            JSON representation of Preprocessor (dict) .
+        """
+        return {self.__class__.__name__: None}
+
 
 class NoOpNormalizer(Normalizer):
     """A No-Ops Normalizer."""
@@ -235,9 +246,11 @@ def __init__(self, pattern: str, replacement: str):
     def normalize(self, s):
         return self._expr.sub(self.replacement, s)
 
+    def tojson(self):
+        return {self.__class__.__name__ + "##" + self.pattern + "##" + self.replacement: None}
 
-class RegexNormalizerRuleFactory:
 
+class RegexNormalizerRuleFactory:
     # exception_chars is a class var so that updates are accessible throughout the application
     EXCEPTION_CHARS = r"\@\[\]'"
 
diff --git a/mindmeld/text_preparation/preprocessors.py b/mindmeld/text_preparation/preprocessors.py
index 31068e621..338dc0be5 100644
--- a/mindmeld/text_preparation/preprocessors.py
+++ b/mindmeld/text_preparation/preprocessors.py
@@ -33,6 +33,18 @@ def process(self, text):
         """
         pass
 
+    def tojson(self):
+        """
+        Method defined to obtain recursive JSON representation of a TextPreparationPipeline.
+
+        Args:
+            None.
+
+        Returns:
+            JSON representation of Preprocessor (dict) .
+        """
+        return {self.__class__.__name__: None}
+
 
 class NoOpPreprocessor(Preprocessor):
     """
diff --git a/mindmeld/text_preparation/stemmers.py b/mindmeld/text_preparation/stemmers.py
index f5c6d1d3c..0f4414ec3 100644
--- a/mindmeld/text_preparation/stemmers.py
+++ b/mindmeld/text_preparation/stemmers.py
@@ -31,6 +31,18 @@ def stem_word(self, word):
         """
         raise NotImplementedError
 
+    def tojson(self):
+        """
+        Method defined to obtain recursive JSON representation of a TextPreparationPipeline.
+
+        Args:
+            None.
+
+        Returns:
+            JSON representation of TextPreparationPipeline (dict) .
+        """
+        return {self.__class__.__name__: None}
+
 
 class NoOpStemmer(Stemmer):
     @property
diff --git a/mindmeld/text_preparation/text_preparation_pipeline.py b/mindmeld/text_preparation/text_preparation_pipeline.py
index 75a9de52b..d7daf5572 100644
--- a/mindmeld/text_preparation/text_preparation_pipeline.py
+++ b/mindmeld/text_preparation/text_preparation_pipeline.py
@@ -16,6 +16,8 @@
 from typing import List, Dict, Tuple, Union
 import re
 import unicodedata
+import json
+from hashlib import sha256
 
 from .normalizers import (
     Normalizer,
@@ -34,10 +36,11 @@
     get_language_config,
     ENGLISH_LANGUAGE_CODE,
 )
-from ..constants import UNICODE_SPACE_CATEGORY
 
-logger = logging.getLogger(__name__)
+from ..constants import UNICODE_SPACE_CATEGORY, DUCKLING_VERSION
+from .._version import get_mm_version
 
+logger = logging.getLogger(__name__)
 
 # Regex Pattern to capture MindMeld entities ("{entity_text|entity_type|optional_role}")
 MINDMELD_ANNOTATION_PATTERN = re.compile(r"\{([^\}\|]*)\|[^\{]*\}")
@@ -192,6 +195,39 @@ def stem_word(self, word):
         """
         return self.stemmer.stem_word(word)
 
+    def tojson(self):
+        """
+        Method defined to obtain recursive JSON representation of a TextPreparationPipeline.
+
+        Args:
+            None.
+
+        Returns:
+            JSON representation of TextPreparationPipeline (dict) .
+        """
+        return {
+            "duckling_version": DUCKLING_VERSION,
+            "mm_version": get_mm_version(),
+            "language": self.language,
+            "preprocessors": self.preprocessors,
+            "normalizers": self.normalizers,
+            "tokenizer": self.tokenizer,
+            "stemmer": self.stemmer
+        }
+
+    def get_hashid(self):
+        """
+        Method defined to obtain Hash value of TextPreparationPipeline.
+
+        Args:
+            None.
+
+        Returns:
+            256 character hash representation of current TextPreparationPipeline config (str) .
+        """
+        string = json.dumps(self, cls=TextPreparationPipelineJSONEncoder, sort_keys=True)
+        return sha256(string.encode()).hexdigest()
+
     @staticmethod
     def find_mindmeld_annotation_re_matches(text):
         """
@@ -246,7 +282,7 @@ def modify_around_annotations(text, function):
 
         if prev_entity_end < len(text):
             # Adds the remainder of the text after the last end brace } "function(post_entity_text)"
-            modified_text.append(function(text[prev_entity_end : len(text)]))
+            modified_text.append(function(text[prev_entity_end: len(text)]))
 
         return "".join(modified_text)
 
@@ -294,7 +330,7 @@ def tokenize_around_mindmeld_annotations(self, text):
         if prev_entity_end < len(text):
             # Add tokens from the text after the last MindMeld entity
             tokens_after_last_entity = self.tokenizer.tokenize(
-                text[prev_entity_end : len(text)]
+                text[prev_entity_end: len(text)]
             )
             TextPreparationPipeline.offset_token_start_values(
                 tokens=tokens_after_last_entity, offset=prev_entity_end
@@ -464,7 +500,7 @@ def create_from_app_path(app_path):
         stemmer = (
             "NoOpStemmer"
             if "stemmer" in text_preparation_config
-            and not text_preparation_config["stemmer"]
+               and not text_preparation_config["stemmer"]
             else text_preparation_config.get("stemmer")
         )
         return TextPreparationPipelineFactory.create_text_preparation_pipeline(
@@ -627,3 +663,23 @@ def _construct_pipeline_component(  # pylint: disable=W0640
             raise TypeError(
                 f"{component} must be of type String or {expected_component_class.__name__}."
             )
+
+
+class TextPreparationPipelineJSONEncoder(json.JSONEncoder):
+    """
+    Custom Encoder class defined to obtain recursive JSON representation of a TextPreparationPipeline.
+
+    Args:
+        None.
+
+    Returns:
+        Custom JSON Encoder class (json.JSONEncoder) .
+    """
+
+    def default(self, o):
+        tojson = getattr(o, 'tojson', None)
+        if callable(tojson):
+            return tojson()
+        else:
+            raise TextPreparationPipelineError(
+                f"Missing tojson() for {o.__class__.__name__} to create query cache hash.")
diff --git a/mindmeld/text_preparation/tokenizers.py b/mindmeld/text_preparation/tokenizers.py
index c93f91c14..f19d10536 100644
--- a/mindmeld/text_preparation/tokenizers.py
+++ b/mindmeld/text_preparation/tokenizers.py
@@ -25,7 +25,6 @@
     SPACY_SUPPORTED_LANGUAGES,
 )
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -46,6 +45,18 @@ def tokenize(self, text):
         """
         raise NotImplementedError("Subclasses must implement this method")
 
+    def tojson(self):
+        """
+        Method defined to obtain recursive JSON representation of a TextPreparationPipeline.
+
+        Args:
+            None.
+
+        Returns:
+            JSON representation of TextPreparationPipeline (dict) .
+        """
+        return {self.__class__.__name__: None}
+
 
 class NoOpTokenizer(Tokenizer):
     """A No-Ops tokenizer."""


Test patch:
diff --git a/tests/test_query_cache.py b/tests/test_query_cache.py
index 76cfa7f69..74eafc433 100644
--- a/tests/test_query_cache.py
+++ b/tests/test_query_cache.py
@@ -13,13 +13,16 @@
 import sqlite3
 from tempfile import TemporaryDirectory
 from unittest.mock import patch
+from hashlib import sha256
 
 from mindmeld.core import ProcessedQuery
 from mindmeld.query_cache import QueryCache
+from mindmeld.text_preparation.text_preparation_pipeline import TextPreparationPipelineFactory
 
 
 def test_query_cache_has_the_correct_format(kwik_e_mart_app_path):
-    cache = QueryCache(kwik_e_mart_app_path)
+    text_prep_pipeline = TextPreparationPipelineFactory.create_from_app_path(kwik_e_mart_app_path)
+    cache = QueryCache(kwik_e_mart_app_path, text_prep_pipeline.get_hashid())
     key = QueryCache.get_key("store_info", "help", "User manual")
     row_id = cache.key_to_row_id(key)
     assert row_id is not None
@@ -46,7 +49,8 @@ def test_disk_query_cache(processed_queries):
     environ = {"MM_QUERY_CACHE_IN_MEMORY": "0"}
 
     with TemporaryDirectory() as tmpdir, patch.dict(os.environ, environ):
-        cache = QueryCache(tmpdir)
+        text_prep_pipeline = TextPreparationPipelineFactory.create_from_app_path(tmpdir)
+        cache = QueryCache(tmpdir, text_prep_pipeline.get_hashid())
 
         # Verify that there is no in-memory caching
         assert cache.memory_connection is None
@@ -64,7 +68,8 @@ def test_memory_query_cache(processed_queries):
     }
 
     with TemporaryDirectory() as tmpdir, patch.dict(os.environ, environ):
-        cache = QueryCache(tmpdir)
+        text_prep_pipeline = TextPreparationPipelineFactory.create_from_app_path(tmpdir)
+        cache = QueryCache(tmpdir, text_prep_pipeline.get_hashid())
 
         # Verify that there is in-memory caching
         assert cache.memory_connection is not None
diff --git a/tests/test_resource_loader.py b/tests/test_resource_loader.py
index baa2dadec..f658aec77 100644
--- a/tests/test_resource_loader.py
+++ b/tests/test_resource_loader.py
@@ -38,7 +38,9 @@ def test_flatten_query_tree(resource_loader):
     assert count == len(flattened)
 
     # verify that query trees built from different query_caches will raise an error
-    qmap["banking"]["transfer_money"].cache = QueryCache(app_path=resource_loader.app_path)
+    hash_id = resource_loader.query_factory.text_preparation_pipeline.get_hashid()
+    qmap["banking"]["transfer_money"].cache = QueryCache(app_path=resource_loader.app_path,
+                                                         schema_version_hash=hash_id)
     with pytest.raises(ValueError):
         flattened = resource_loader.flatten_query_tree(qmap)
 
diff --git a/tests/text_preparation/test_text_preparation_pipeline.py b/tests/text_preparation/test_text_preparation_pipeline.py
index e61a56329..722ffca20 100644
--- a/tests/text_preparation/test_text_preparation_pipeline.py
+++ b/tests/text_preparation/test_text_preparation_pipeline.py
@@ -40,7 +40,6 @@ def test_offset_token_start_values():
 
 
 def test_filter_out_space_text_tokens():
-
     input_tokens = [
         {"text": "How"},
         {"text": "   "},
@@ -60,7 +59,6 @@ def test_filter_out_space_text_tokens():
 
 
 def test_find_mindmeld_annotation_re_matches():
-
     sentence = "Hello {Lucien|sys_person|employee}. Do you have {1|sys_number} cat?"
     matches = TextPreparationPipeline.find_mindmeld_annotation_re_matches(sentence)
     assert len(matches) == 2
@@ -72,7 +70,6 @@ def test_find_mindmeld_annotation_re_matches():
 
 
 def test_normalize_around_annoations():
-
     sentence = "HELLO {LUCIEN|PERSON_NAME}, HOW ARE YOU?"
     normalized_sentence = TextPreparationPipeline.modify_around_annotations(
         text=sentence, function=Lowercase().normalize
@@ -82,7 +79,6 @@ def test_normalize_around_annoations():
 
 
 def test_tokenize_around_annoations():
-
     text_preparation_pipeline = (
         TextPreparationPipelineFactory.create_default_text_preparation_pipeline()
     )
@@ -101,7 +97,6 @@ def test_tokenize_around_annoations():
 
 
 def test_create_text_preparation_pipeline():
-
     text_preparation_pipeline = (
         TextPreparationPipelineFactory.create_text_preparation_pipeline(
             language=ENGLISH_LANGUAGE_CODE,
@@ -122,8 +117,70 @@ def test_create_text_preparation_pipeline():
     assert isinstance(text_preparation_pipeline.stemmer, EnglishNLTKStemmer)
 
 
-def test_construct_pipeline_components_valid_input():
+def test_text_preparation_pipeline_hash():
+    text_preparation_pipeline = (
+        TextPreparationPipelineFactory.create_text_preparation_pipeline(
+            language=ENGLISH_LANGUAGE_CODE,
+            preprocessors=["NoOpPreprocessor"],
+            regex_norm_rules=[{"pattern": ".*", "replacement": "cisco"}],
+            normalizers=["Lowercase", "ASCIIFold"],
+            tokenizer="WhiteSpaceTokenizer",
+            stemmer=None,
+        )
+    )
 
+    original_hash = text_preparation_pipeline.get_hashid()
+
+    # Change order of normalizers
+    text_preparation_pipeline = (
+        TextPreparationPipelineFactory.create_text_preparation_pipeline(
+            language=ENGLISH_LANGUAGE_CODE,
+            preprocessors=["NoOpPreprocessor"],
+            regex_norm_rules=[{"pattern": ".*", "replacement": "cisco"}],
+            normalizers=["ASCIIFold", "Lowercase"],
+            tokenizer="WhiteSpaceTokenizer",
+            stemmer=None,
+        )
+    )
+    order_changed_hash = text_preparation_pipeline.get_hashid()
+
+    # Change RegexNormalizer pattern
+    text_preparation_pipeline = (
+        TextPreparationPipelineFactory.create_text_preparation_pipeline(
+            language=ENGLISH_LANGUAGE_CODE,
+            preprocessors=["NoOpPreprocessor"],
+            regex_norm_rules=[{"pattern": ".*", "replacement": "cisc0"}],
+            normalizers=["ASCIIFold", "Lowercase"],
+            tokenizer="WhiteSpaceTokenizer",
+            stemmer=None,
+        )
+    )
+
+    regex_changed_hash = text_preparation_pipeline.get_hashid()
+
+    # Change Tokenizer type
+    text_preparation_pipeline = (
+        TextPreparationPipelineFactory.create_text_preparation_pipeline(
+            language=ENGLISH_LANGUAGE_CODE,
+            preprocessors=["NoOpPreprocessor"],
+            regex_norm_rules=[{"pattern": ".*", "replacement": "cisco"}],
+            normalizers=["ASCIIFold", "Lowercase"],
+            tokenizer="LetterTokenizer",
+            stemmer=None,
+        )
+    )
+
+    tokenizer_changed_hash = text_preparation_pipeline.get_hashid()
+
+    assert original_hash != order_changed_hash
+    assert original_hash != regex_changed_hash
+    assert original_hash != tokenizer_changed_hash
+    assert tokenizer_changed_hash != regex_changed_hash
+    assert tokenizer_changed_hash != order_changed_hash
+    assert regex_changed_hash != order_changed_hash
+
+
+def test_construct_pipeline_components_valid_input():
     text_preparation_pipeline = (
         TextPreparationPipelineFactory.create_text_preparation_pipeline(
             preprocessors=("NoOpPreprocessor", NoOpPreprocessor()),
@@ -148,7 +205,6 @@ def test_construct_pipeline_components_valid_input():
 
 
 def test_construct_pipeline_components_invalid_input():
-
     with pytest.raises(TypeError):
         TextPreparationPipelineFactory.create_text_preparation_pipeline(
             preprocessors=None,