Change --data type in 'rasa data validate' command

[martasls]

Proposed changes:

• Change "--data" type in the 'rasa data validate' command to allow more than one path to be passed

Status (please check what you already did):

• reformat files using black (please check Readme for instructions)

[CLAassistant]

All committers have signed the CLA.

[martasls]

@ArjaanBuijk
Opened the PR. Could you please assign yourself as reviewer as it doesn't look like I can do it from my end. Thanks!!

[sara-tagger]

Thanks for opening a draft pull request 🚀If you have any questions, you can direct them to @JustinaPetr ✨

[ArjaanBuijk]

@martasls ,
I tested your updates and they work perfect.

I think you still have to sign the Contributor License Agreement (see the link above), which must be done before we can merge it.

[ArjaanBuijk]

@wochinge ,
Can you have a look at the updates as well?

It all looks good to me and I tested the updates, but wanted to make sure before I merge it in.

[ArjaanBuijk]

@martasls ,
thanks for signing the CLA !

[wochinge]

Thanks for the contribution! Can you please add a changelog entry?

[martasls]

Thanks for the contribution! Can you please add a changelog entry?

Hi @wochinge, should I enter an entry under the latest version?

[ArjaanBuijk]

@wochinge ,
do you know why the Continuous Integration / Wait for docs test (pull_request) is failing ?

[wochinge]

Hi @wochinge, should I enter an entry under the latest version?

A new file called 8225.improvemend.md in the changelog folder is sufficient (see the Readme here)

do you know why the Continuous Integration / Wait for docs test (pull_request) is failing ?

tbh no, but it's not required for the merge afaik.

[ArjaanBuijk]

@martasls ,
the cli unit tests are failing. Can you please fix them?

Eg. see these Details

[martasls]

@martasls ,
thank you for fixing those tests. We're getting closer!

I noticed some failed tests unrelated to your updates, so I did another merge.

If that does not fix it, I will ask our QA team for help with those.

Hi Arjaan,

it does look like those failing tests are still related with converting files (using the data command). I'll try to have a look tomorrow!

[martasls]

Hi @ArjaanBuijk , could you approve the workflow? Thanks!

[mprazz]

@ArjaanBuijk As far as I can see from the logs, the failures are related to the changes, or am I missing something? The errors mention the --data argument.

[martasls]

@ArjaanBuijk As far as I can see from the logs, the failures are related to the changes, or am I missing something? The errors mention the --data argument.

Hi @mprazz,
you're right. I've already looked into it and I'm waiting for the workflow to be approved to see if the tests are succeeding now.

[martasls]

Hi @ArjaanBuijk, test-full-model-training is failing on Windows however it looks like it's related to some package not being installed?

E          rasa.exceptions.MissingDependencyException: Not all required importable packages are installed to use the configured NLU pipeline. To use this pipeline, you need to install the missing modules: 
E             - transformers (needed for HFTransformersNLP, LanguageModelFeaturizer)
E           Please install the packages that contain the missing modules.

[martasls]

Hi @ArjaanBuijk, only the Code Quality test is failing now. I went ahead and added docstrings for the convert_training_data method which was causing the test to fail.

[ArjaanBuijk]

@martasls ,

I went through everything in detail again, and it all looks great. Awesome job.

I noticed that there is a new binary file called other . Was this checked in by accident?

I saw it was added with commit 2a9b90: Fix data path in convert data method.

[martasls]

@martasls ,

I went through everything in detail again, and it all looks great. Awesome job.

I noticed that there is a new binary file called other . Was this checked in by accident?

I saw it was added with commit 2a9b90: Fix data path in convert data method.

Done!

[ArjaanBuijk]

@martasls ,
nice!
once tests pass, it should be ready to go.

[martasls]

@martasls ,
nice!
once tests pass, it should be ready to go.

Ah sorry, one last commit (hopefully). There was a blank line after the docstring.

[ArjaanBuijk]

@martasls ,
seems like there is still a small mypy issue in the convert.py.

Note that you can run that test locally as well with:

$ make types
rasa/nlu/convert.py:28: error: Value of type variable "AnyStr" of "exists" cannot be "Sequence[Any]"  [type-var]
Found 1 error in 1 file (checked 258 source files)
make: *** [Makefile:105: types] Error 1

[martasls]

@martasls ,
seems like there is still a small mypy issue in the convert.py.

Note that you can run that test locally as well with:

$ make types
rasa/nlu/convert.py:28: error: Value of type variable "AnyStr" of "exists" cannot be "Sequence[Any]"  [type-var]
Found 1 error in 1 file (checked 258 source files)
make: *** [Makefile:105: types] Error 1

Thanks. I wasn't aware of this mypy test. Hope it's fixed now (I ran it locally and it seemed ok)!

[ArjaanBuijk]

@martasls ,

It all passed and it is merged!

Thanks you so much for your contribution and getting it updated to pass all these tests.

[martasls]

@martasls ,

It all passed and it is merged!

Thanks you so much for your contribution and getting it updated to pass all these tests.

Thanks for the help and availability!