Repository: RasaHQ/rasa
Pull request: Do not dump untrainable entities to markdown
**Proposed changes**:
- fixes #4877 

**Status (please check what you already did)**:
- [x] added some tests for the functionality
- [x] updated the documentation
- [x] updated the changelog (please check [changelog](https://github.com/RasaHQ/rasa/tree/master/changelog) for instructions)
- [x] reformat files using `black` (please check [Readme](https://github.com/RasaHQ/rasa#code-style) for instructions)


Problem statement:
MarkdownWriter shouldn't dump pre-trained entities
I noticed this on Rasa X, that if duckling or ner_spacy entities are recognised and you save it, these entities will get saved to the markdown file. These entities should be filtered out since we don't want to train the crf on it. 

I'm not sure how we persist this training data to the database on Rasa X, but there might be something we need to change there too

Hints from issue discussion:
@akelad these entities aren't used for training of the CRF: https://github.com/RasaHQ/rasa/pull/898

I think dumping them should be okay then, no? we explicitly removed this from rasa X at some point ages ago: https://github.com/RasaHQ/nlu-trainer-backend/pull/42
@ricwo how does that work? We just exported the training data from the instance to use it in the repo, and it was exported with duckling entities annotated as CRF ones:
```
- [$0.00](amount-of-money:0)
- [$1000](amount-of-money:1000)
- [0](number)
```
`rasa` will think those are CRF entities won't it?
synonyms from duckling get dumped as well:
```
## synonym:1
- SINGLE
- one
- single
```
does not look to be duckling-specific:
```
- [BCBSM](company:bcbsm)[BCBSM](ORG)
- [Helvetia](company:helvetia)[Helvetia](GPE)
```

Not sure if this is related. It seems that by default the case insensitivity is leading to synonym dumps for entities that have capitals in them:
```
## synonym:jenny
- Jenny

## synonym:jim
- Jim

## synonym:jim halpert
- Jim Halpert

## synonym:john smith
- John Smith
```

Commit message: Merge branch 'md-dump-trainable-entities' of https://github.com/RasaHQ/rasa into md-dump-trainable-entities

Patch to review:
diff --git a/changelog/4877.improvement.rst b/changelog/4877.improvement.rst
new file mode 100644
index 000000000000..ec06e18ddef6
--- /dev/null
+++ b/changelog/4877.improvement.rst
@@ -0,0 +1,3 @@
+Entity examples that have been extracted using an external extractor are excluded
+from Markdown dumping in ``MarkdownWriter.dumps()``. The excluded external extractors
+are ``DucklingHTTPExtractor``, ``MitieEntityExtractor`` and ``SpacyEntityExtractor``.
\ No newline at end of file
diff --git a/rasa/core/training/interactive.py b/rasa/core/training/interactive.py
index 9909132e521b..e8b9968a0eb4 100644
--- a/rasa/core/training/interactive.py
+++ b/rasa/core/training/interactive.py
@@ -11,6 +11,7 @@
 import numpy as np
 from aiohttp import ClientError
 from colorclass import Color
+
 from rasa.nlu.training_data.loading import MARKDOWN, RASA
 from sanic import Sanic, response
 from sanic.exceptions import NotFound
@@ -727,35 +728,17 @@ def _split_conversation_at_restarts(
 def _collect_messages(events: List[Dict[Text, Any]]) -> List[Message]:
     """Collect the message text and parsed data from the UserMessage events
     into a list"""
-    from rasa.nlu.extractors.duckling_http_extractor import DucklingHTTPExtractor
-    from rasa.nlu.extractors.mitie_entity_extractor import MitieEntityExtractor
-    from rasa.nlu.extractors.spacy_entity_extractor import SpacyEntityExtractor
+
+    import rasa.nlu.training_data.util as rasa_nlu_training_data_utils
 
     messages = []
 
     for event in events:
         if event.get("event") == UserUttered.type_name:
             data = event.get("parse_data", {})
-
-            for entity in data.get("entities", []):
-
-                excluded_extractors = [
-                    DucklingHTTPExtractor.__name__,
-                    SpacyEntityExtractor.__name__,
-                    MitieEntityExtractor.__name__,
-                ]
-                logger.debug(
-                    f"Exclude entity marking of following extractors "
-                    f"{excluded_extractors} when writing nlu data "
-                    f"to file."
-                )
-
-                if entity.get("extractor") in excluded_extractors:
-                    data["entities"].remove(entity)
-
+            rasa_nlu_training_data_utils.remove_untrainable_entities_from(data)
             msg = Message.build(data["text"], data["intent"]["name"], data["entities"])
             messages.append(msg)
-
         elif event.get("event") == UserUtteranceReverted.type_name and messages:
             messages.pop()  # user corrected the nlu, remove incorrect example
 
diff --git a/rasa/nlu/constants.py b/rasa/nlu/constants.py
index 6fb2f023fe18..9bbafd8cfe2d 100644
--- a/rasa/nlu/constants.py
+++ b/rasa/nlu/constants.py
@@ -8,6 +8,8 @@
 
 ENTITIES_ATTRIBUTE = "entities"
 
+EXTRACTOR_ATTRIBUTE = "extractor"
+
 CLS_TOKEN = "__CLS__"
 
 MESSAGE_ATTRIBUTES = [TEXT_ATTRIBUTE, INTENT_ATTRIBUTE, RESPONSE_ATTRIBUTE]
diff --git a/rasa/nlu/extractors/__init__.py b/rasa/nlu/extractors/__init__.py
index 537705e481a5..fd6cad6e3e4c 100644
--- a/rasa/nlu/extractors/__init__.py
+++ b/rasa/nlu/extractors/__init__.py
@@ -1,6 +1,7 @@
 from typing import Any, Dict, List, Text, Tuple
 
 from rasa.nlu.components import Component
+from rasa.nlu.constants import EXTRACTOR_ATTRIBUTE, ENTITIES_ATTRIBUTE
 from rasa.nlu.training_data import Message
 
 
@@ -9,7 +10,7 @@ def add_extractor_name(
         self, entities: List[Dict[Text, Any]]
     ) -> List[Dict[Text, Any]]:
         for entity in entities:
-            entity["extractor"] = self.name
+            entity[EXTRACTOR_ATTRIBUTE] = self.name
         return entities
 
     def add_processor_name(self, entity: Dict[Text, Any]) -> Dict[Text, Any]:
@@ -71,12 +72,12 @@ def filter_trainable_entities(
         filtered = []
         for message in entity_examples:
             entities = []
-            for ent in message.get("entities", []):
-                extractor = ent.get("extractor")
+            for ent in message.get(ENTITIES_ATTRIBUTE, []):
+                extractor = ent.get(EXTRACTOR_ATTRIBUTE)
                 if not extractor or extractor == self.name:
                     entities.append(ent)
             data = message.data.copy()
-            data["entities"] = entities
+            data[ENTITIES_ATTRIBUTE] = entities
             filtered.append(
                 Message(
                     text=message.text,
diff --git a/rasa/nlu/test.py b/rasa/nlu/test.py
index a6cfee1fcb30..b3ba2e63e887 100644
--- a/rasa/nlu/test.py
+++ b/rasa/nlu/test.py
@@ -25,6 +25,7 @@
     DEFAULT_OPEN_UTTERANCE_TYPE,
     RESPONSE_SELECTOR_PROPERTY_NAME,
     OPEN_UTTERANCE_PREDICTION_KEY,
+    EXTRACTOR_ATTRIBUTE,
 )
 from rasa.model import get_model
 from rasa.nlu import config, training_data, utils
@@ -873,7 +874,7 @@ def align_entity_predictions(
         extractor: [] for extractor in extractors
     }
     for p in result.entity_predictions:
-        entities_by_extractors[p["extractor"]].append(p)
+        entities_by_extractors[p[EXTRACTOR_ATTRIBUTE]].append(p)
     extractor_labels: Dict[Text, List] = {extractor: [] for extractor in extractors}
     for t in result.tokens:
         true_token_labels.append(determine_token_labels(t, result.entity_targets, None))
diff --git a/rasa/nlu/training_data/formats/markdown.py b/rasa/nlu/training_data/formats/markdown.py
index e62f5743e7a2..3b27facce2cb 100644
--- a/rasa/nlu/training_data/formats/markdown.py
+++ b/rasa/nlu/training_data/formats/markdown.py
@@ -11,11 +11,8 @@
     TrainingDataWriter,
 )
 from rasa.nlu.utils import build_entity
-from rasa.nlu.constants import (
-    INTENT_ATTRIBUTE,
-    RESPONSE_KEY_ATTRIBUTE,
-    RESPONSE_IDENTIFIER_DELIMITER,
-)
+from rasa.nlu.constants import INTENT_ATTRIBUTE
+
 
 if typing.TYPE_CHECKING:
     from rasa.nlu.training_data import Message, TrainingData
@@ -214,10 +211,13 @@ def dumps(self, training_data: "TrainingData") -> Text:
     def _generate_training_examples_md(self, training_data: "TrainingData") -> Text:
         """Generates markdown training examples."""
 
+        import rasa.nlu.training_data.util as rasa_nlu_training_data_utils
+
         training_examples = OrderedDict()
 
         # Sort by intent while keeping basic intent order
         for example in [e.as_dict_nlu() for e in training_data.training_examples]:
+            rasa_nlu_training_data_utils.remove_untrainable_entities_from(example)
             intent = example[INTENT_ATTRIBUTE]
             training_examples.setdefault(intent, [])
             training_examples[intent].append(example)
diff --git a/rasa/nlu/training_data/util.py b/rasa/nlu/training_data/util.py
index 8b885349a630..af894d07b732 100644
--- a/rasa/nlu/training_data/util.py
+++ b/rasa/nlu/training_data/util.py
@@ -1,9 +1,11 @@
+import json
 import logging
 import os
 import warnings
 from typing import Any, Dict, Optional, Text
 
 import rasa.utils.io as io_utils
+from rasa.nlu.constants import ENTITIES_ATTRIBUTE, EXTRACTOR_ATTRIBUTE
 
 logger = logging.getLogger(__name__)
 
@@ -49,3 +51,45 @@ def get_file_format(resource_name: Text) -> Text:
         return fformat
 
     return "json"
+
+
+def remove_untrainable_entities_from(example: Dict[Text, Any]) -> None:
+    """Remove untrainable entities from serialised training example `example`.
+
+    Entities with an untrainable extractor will be removed. Untrainable extractors
+    are `DucklingHTTPExtractor`, `MitieEntityExtractor` and `SpacyEntityExtractor`.
+
+    Args:
+        example: Serialised training example to inspect.
+    """
+
+    from rasa.nlu.extractors.duckling_http_extractor import DucklingHTTPExtractor
+    from rasa.nlu.extractors.mitie_entity_extractor import MitieEntityExtractor
+    from rasa.nlu.extractors.spacy_entity_extractor import SpacyEntityExtractor
+
+    untrainable_entity_extractors = [
+        DucklingHTTPExtractor.__name__,
+        SpacyEntityExtractor.__name__,
+        MitieEntityExtractor.__name__,
+    ]
+
+    example_entities = example.get(ENTITIES_ATTRIBUTE)
+
+    if not example_entities:
+        # example contains no entities, so there's nothing to do
+        return None
+
+    trainable_entities = []
+
+    for entity in example_entities:
+        if entity.get(EXTRACTOR_ATTRIBUTE) in untrainable_entity_extractors:
+            logger.debug(
+                f"Excluding entity '{json.dumps(entity)}' from training data. "
+                f"Entity examples extracted by the following classes are not "
+                f"dumped to training data in markdown format: "
+                f"`{'`, `'.join(untrainable_entity_extractors)}`."
+            )
+        else:
+            trainable_entities.append(entity)
+
+    example[ENTITIES_ATTRIBUTE] = trainable_entities
diff --git a/tests/nlu/base/test_training_data.py b/tests/nlu/base/test_training_data.py
index 23e9437c5808..880d2e3b5d88 100644
--- a/tests/nlu/base/test_training_data.py
+++ b/tests/nlu/base/test_training_data.py
@@ -1,14 +1,20 @@
+import logging
+from typing import Optional, Text
+
 import pytest
 import tempfile
 from jsonschema import ValidationError
 
 from rasa.nlu import training_data
 from rasa.nlu.convert import convert_training_data
+from rasa.nlu.extractors.crf_entity_extractor import CRFEntityExtractor
+from rasa.nlu.extractors.duckling_http_extractor import DucklingHTTPExtractor
 from rasa.nlu.extractors.mitie_entity_extractor import MitieEntityExtractor
+from rasa.nlu.extractors.spacy_entity_extractor import SpacyEntityExtractor
 from rasa.nlu.tokenizers.whitespace_tokenizer import WhitespaceTokenizer
 from rasa.nlu.training_data import TrainingData
-from rasa.nlu.training_data.formats import MarkdownReader
-from rasa.nlu.training_data.formats.rasa import validate_rasa_nlu_data
+from rasa.nlu.training_data.formats import MarkdownReader, MarkdownWriter
+from rasa.nlu.training_data.formats.rasa import validate_rasa_nlu_data, RasaReader
 from rasa.nlu.training_data.loading import guess_format, UNK, load_data
 from rasa.nlu.training_data.util import get_file_format
 import rasa.utils.io as io_utils
@@ -660,3 +666,41 @@ def test_dump_nlu_with_responses():
 
     dumped = nlu_data.nlu_as_markdown()
     assert dumped == md
+
+
+@pytest.mark.parametrize(
+    "entity_extractor,expected_output",
+    [
+        (None, "- [test](word:random)"),
+        ("", "- [test](word:random)"),
+        ("random-extractor", "- [test](word:random)"),
+        (CRFEntityExtractor.__name__, "- [test](word:random)"),
+        (DucklingHTTPExtractor.__name__, "- test"),
+        (SpacyEntityExtractor.__name__, "- test"),
+        (MitieEntityExtractor.__name__, "- test"),
+    ],
+)
+def test_dump_trainable_entities(
+    entity_extractor: Optional[Text], expected_output: Text
+):
+    training_data_json = {
+        "rasa_nlu_data": {
+            "common_examples": [
+                {
+                    "text": "test",
+                    "intent": "greet",
+                    "entities": [
+                        {"start": 0, "end": 4, "value": "random", "entity": "word"}
+                    ],
+                }
+            ]
+        }
+    }
+    if entity_extractor is not None:
+        training_data_json["rasa_nlu_data"]["common_examples"][0]["entities"][0][
+            "extractor"
+        ] = entity_extractor
+
+    training_data_object = RasaReader().read_from_json(training_data_json)
+    md_dump = MarkdownWriter().dumps(training_data_object)
+    assert md_dump.splitlines()[1] == expected_output


Code review comments:
diff_hunk: @@ -49,3 +51,45 @@ def get_file_format(resource_name: Text) -> Text:
         return fformat
 
     return "json"
+
+
+def remove_untrainable_entities_from(example: Dict[Text, Any]) -> None:
+    """Remove untrainable entities from serialised training example `example`.
+
+    Entities with an untrainable extractor will be removed. Untrainable extractors
+    are `DucklingHTTPExtractor`, `MitieEntityExtractor` and `SpacyEntityExtractor`.
path: rasa/nlu/training_data/util.py
text: @user1:
Why do we remove entities from the MitieEntityExtractor?

@author:
good point, shouldn't be removed!

diff_hunk: @@ -49,3 +51,45 @@ def get_file_format(resource_name: Text) -> Text:
         return fformat
 
     return "json"
+
+
+def remove_untrainable_entities_from(example: Dict[Text, Any]) -> None:
+    """Remove untrainable entities from serialised training example `example`.
+
+    Entities with an untrainable extractor will be removed. Untrainable extractors
+    are `DucklingHTTPExtractor`, `MitieEntityExtractor` and `SpacyEntityExtractor`.
+
+    Args:
+        example: Serialised training example to inspect.
+    """
+
+    from rasa.nlu.extractors.duckling_http_extractor import DucklingHTTPExtractor
+    from rasa.nlu.extractors.mitie_entity_extractor import MitieEntityExtractor
+    from rasa.nlu.extractors.spacy_entity_extractor import SpacyEntityExtractor
+
+    untrainable_entity_extractors = [
+        DucklingHTTPExtractor.__name__,
+        SpacyEntityExtractor.__name__,
+        MitieEntityExtractor.__name__,
+    ]
path: rasa/nlu/training_data/util.py
text: @user1:
can we make one list of untrainable extractors that we use anywhere necessary? https://github.com/RasaHQ/rasa/blob/master/rasa/nlu/test.py#L41

@author:
good point, we should just use the one in `test.py` (and move it to constants.py)