Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Add Vocab File Auto test if language is set to a value other than en-us #716

Open
gumulka opened this issue Dec 23, 2018 · 6 comments

Comments

@gumulka
Copy link

gumulka commented Dec 23, 2018

Partially Issue, Partially Feature Request.

History

Today I switched Mycroft to german (de-de), and even though I know it is experimental, it still works fine for most parts. I found some bugs, some wording errors, but also I found the following issue in the log:

Issue

Some Skills failed to load, because not all vocab, regex or intent files are present. One Example:

14:38:50.880 - mycroft.skills.core:load_skill:159 - ERROR - Failed to load skill: mycroft-personal.mycroftai
Traceback (most recent call last):
  File "/home/pi/mycroft-core/mycroft/skills/core.py", line 143, in load_skill
    raise e
  File "/home/pi/mycroft-core/mycroft/skills/core.py", line 138, in load_skill
    skill._register_decorated()
  File "/home/pi/mycroft-core/mycroft/skills/core.py", line 688, in _register_decorated
    self.register_intent_file(intent_file, method)
  File "/home/pi/mycroft-core/mycroft/skills/core.py", line 975, in register_intent_file
    raise ValueError('Unable to find "' + str(intent_file) + '"')
ValueError: Unable to find "DoYouRhyme.intent"

Same goes for mycroft-reminder.

Feature Request

Add a test to check if all language subdirectories contain the same files.
So basically write a Unittest, that goes through all subdirectories, checks if there are localisation files present (de-de, en-us, ....) and fails if some of them do not contain the same set of files as all the other.

@KathyReid KathyReid changed the title Vocab File Auto test Feature request: Add Vocab File Auto test if language is set to a value other than en-us Dec 26, 2018
@KathyReid
Copy link
Contributor

Thanks for your feature request @gumulka, I really appreciate you taking the time to submit it.
I'm going to flag this with my colleague @forslund to see if he can give an estimate of how easy / how difficult this would be to implement.

@gumulka
Copy link
Author

gumulka commented Dec 26, 2018

Thanks for the answer, but the title should be a little bit different.

Unittest should always ensure, that all language directories contain the same files.

let me explain it with an example:
Folder structure 1:

vocab
+-en-us
|   +- startsomething.voc
|   +- working.voc
+-de-de
|   +- startsomething.voc
|   +- working.voc
+-it-it
    +- startsomething.voc
    +- working.voc

^^^^ this is how a folder structure should look like

Folder structure 2:

vocab
+-en-us
|   +- start.voc
|   +- working.voc
+-de-de
|   +- startsomething.voc
|   +- working.voc
+-it-it
    +- startsomething.voc
    +- work.voc

^^^ This can only work in one language and therefore a test should fail, ensuring, that all language directories, if they exist contain the same files. (as in filenames, not content)

Your test only test the english version and ensure, that a file with the content is present and they get somehow parsed. But in different languages, the skills cannot load, because files are missing.

I know how one sometimes changes files or add some to expand the scope of the skill, but when doing so, one should not break the workflow for everyone else, or at least something should tell it to the person/developer.

@forslund
Copy link
Collaborator

Hi, this should be quite easy to implement but there may be sort of a catch 22 here...

(Especially at the moment when the skills need to be accepted into the mycroft-skills repo before the translation server is updated with new phrases.)

I'm also worried that having this as a rejection criteria will be detrimental to language adoption since the skill could be rejected due to a single language not getting updated. Or am I misreading the intent here?

What may be an alternative is to have separate branches for each language mirroring the main one but only updating the commit reference if the language of the branch is complete. This means a working skill would never stop working just because a particular language is lagging behind.

@gumulka
Copy link
Author

gumulka commented Dec 31, 2018

Maybe don't make it a rejection criteria, but flag a warning to show the user, that there is something missing, or possibly not working correctly.

When writing this issue, I was assuming, that a developer must update the language files, or leave them out completely and not, that all language files are generated by the translation server.

I did not yet understand completely how it works with the translation server, so I'm going to leave it as it is and let you decide what to do or not.

@LinusSkucas
Copy link
Contributor

Maybe only export the translated files to the skill after the skill is fully translated?

@krisgesling
Copy link
Contributor

That is the current behaviour. It should only export translations for a Skill if it believes they are complete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants