Remove Markdown training data format #9390
Conversation
|
One question I've had that isn't only related to this but also other backwards compatible changes in As an example, Rasa X imports |
|
@chdorner yes, that's correct — we're currently introducing a lot of architectural changes so it's not guaranteed that Rasa X will work with Rasa from |
alwx
requested review from
a team,
alopez and
wochinge
and removed request for
a team
There was a problem hiding this comment.
Wow - this is big! Thanks for taking this huge thing on 💯
Some other MD things I found
- comment here, here and some others
- do a lot of the
as_story_stringmethods actually still make sense? I think we need to keep theEvent.as_story_stringones forrasa interactivebut we might be able to delete them forStoryGraph,Story(Rasa X is using this here but this doesn't make sense any more after dropping MD in Rasa X as well). Might be worth dropping that in a separate PR - drop it here
| @@ -0,0 +1,5 @@ | |||
| Remove the support of Markdown training data format. This includes: | |||
There was a problem hiding this comment.
I'd add also an entry to the migration guide and maybe a link to the old migration guide where we tell them how to convert old MD files to YAML
tests/shared/core/training_data/story_reader/test_common_story_reader.py
Outdated
Show resolved
Hide resolved
| @@ -62,18 +56,6 @@ def read_from_file( | |||
| """ | |||
| raise NotImplementedError | |||
|
|
|||
| @staticmethod | |||
| def is_test_stories_file(filename: Text) -> bool: | |||
There was a problem hiding this comment.
I'm slightly confused why this method was removed 🤔
There was a problem hiding this comment.
No reason for it to exist because it's just a static method that just proxies its arguments to another function.
There was a problem hiding this comment.
Ah I see, you're referring to its implementation in YAMLStoryReader.
| test_file = test_dir / f"tests{suffix}" | ||
| test_file.write_bytes(request.body) | ||
| return str(test_file) | ||
| def _test_data_file_from_payload(request: Request, temporary_directory: Path) -> Text: |
changelog/9390.removal.md
Outdated
| @@ -0,0 +1,2 @@ | |||
| Removes `template_variables` argument from `get_stories` method of `TrainingDataImporter`. | |||
There was a problem hiding this comment.
| Removes `template_variables` argument from `get_stories` method of `TrainingDataImporter`. | |
| Removes `template_variables` and `e2e` arguments from `get_stories` method of `TrainingDataImporter`. |
There was a problem hiding this comment.
I'd also mention in the migration guide as it's an explicit public interface
| test_file = test_dir / f"tests{suffix}" | ||
| test_file.write_bytes(request.body) | ||
| return str(test_file) | ||
| def _test_data_file_from_payload(request: Request, temporary_directory: Path) -> Text: |
There was a problem hiding this comment.
@alwx _training_payload_from_json is still there.
Proposed changes:
Scope/list of changes:
_convert_core_datafunction and the ability to convert data from markdown to yamltest_yaml_story_writer.pyto work without a MarkdownStoryReadertest_extractor.pyto useRasaYAMLReaderinstead ofMarkdownReader_convert_nlg_datafunction and the ability to convert data from NLG markdown to yamlrasa.nlu.training_data.convertersdatadirectoryuse_e2ein Rasa Open Source 3.0.0 when removing Markdown supportStatus (please check what you already did):
black(please check Readme for instructions)