MycroftAI / mycroft-timer

Mycroft AI official Timer Skill - set multiple named timers
https://mycroft.ai/skills
Apache License 2.0
7 stars 30 forks source link

Adding german dialog files for proper translation and processing #77

Closed emphasize closed 3 years ago

emphasize commented 4 years ago

There are several german dialog files missing which absence leads to half english half german sentences and confusion in the process

forslund commented 4 years ago

Hmm, this looks a bit weird to me. Those ".word" files should have been found in the mycroft-core res/de-de/text folder. so may be a core bug...

Can you provide an example of a failing scenario?

emphasize commented 4 years ago

Why weird? These are the .list files you can find in about every skill-timer/dialog/lang/ folder _translate_word(name, lang) natively takes .word files (i find ".word" more descriptive than list).

The case would be (in german) starte timer für 15 minuten 30 sekunden the output would be (without the fix) Okay, ich habe einen Timer für 15 minutes 30 seconds eingestellt

Positivly tested

emphasize commented 4 years ago

That being said, i was under the impression that self.translate_list('no') also takes .word files. If not, these must be changed (back in my mind i can remember that the ending is completely irrelevant, but..)

forslund commented 4 years ago

I don't doubt that the skill does the wrong thing, I'm just trying to pinpoint where the error is, if these files should need to be here or if the translations in core or lingua-franca should have been used, and this is showing a deeper issue.

What I thought was weird is that (for example) since the hour.word is defined (and translated) in mycroft-core AND in lingua-franca it should be using that without adding the hour.word to this specific skill. If the word files needs to be in the skill then they should be added to the English as well so they're picked up by translate.mycroft.ai...

From a quick glance I don't think translate_list takes .word files as well as .list files (but the glance was quick so I may be mistaken)

emphasize commented 4 years ago

... AND in lingua-franca ...

Does it? The way i see it (and my other PR only tackles the parsing side) duration handeling (especially formatting) is only made in english - nothing points to lingua franca in terms of foreign duration handeling.

forslund commented 4 years ago

Lingua franca has support for nice_duration in german:

>>> nice_duration(dur)
'one hour'
>>> nice_duration(dur, lang='de-de')
'eins Stunde'

One possibility could be that nice_duration() is called without the lang=self.lang argument...But it should use the default language which should be de-de in your case

Edit: not quite sure "Eins" is the correct form...

emphasize commented 4 years ago

should be "eine" (and this should be tackled by nice_daration_de (which dont exist, ...yet))

forslund commented 4 years ago

That's what my few memories of my German classes from high school suggested as well :)

I'm going to check on the handling of "default language" tomorrow since that may be a general issue in core.

forslund commented 4 years ago

Hmmm something is strange, using your PR branch for lingua_franca and fixing core to not only use English I seem to get the German response without adding these word files to the skill...

starte timer für 15 minuten 30
>> Timer gestartet für 15 Minuten dreißig Sekunden

I wonder what's the difference here...

Edit: I realize I used text to enter the language, if an english STT is used that could switch the language code. Did you do the test with voice?

Edit 2: Looking at the code updating the language setting in core it seems like if the STT config doesn't contain an explicit language it will change the default language to 'en-us' and not the configured language...

emphasize commented 4 years ago

Predominantly test with voice in live setup.

No STT (unlike TTS) is set explicitly, since Google is the deafult and i thought the language is inherited by "lang"="de-de" in that case, but didn't double check mycroft-config. This is what your PR adresses, right?

I wonder what's the difference here...

because of "dreißig"? (format: nice_duration/nice_number_de)

May i suggest: If the files shouldn't be needed to remove these alltogether from dialog/lang/-folders. I think many contributions are adaptions at first

forslund commented 4 years ago

If no STT is set it should report the main language buuut something is weird here...

Using voice I get like you no translated minutes

starte einen timer für 15 minuten
 >> Timer gestartet für fünfzehn minutes

With english "minutes"

When using the text client (which uses the lang code directly from the config)

starte einen timer für 15 minuten
>> Timer gestartet für fünfzehn Minuten

with german "Minuten"

So mycroft core does indeed do something strange when the STT is the one sending the info...

forslund commented 4 years ago

The difference I found was that the stt.lang code was sent as de-DE and making that lower case seems to have fixed the issue with english units instead of german units. Pushed this to the fix of default language branch and using that together with your lingua-franca change I get (what I think is correct German output).

emphasize commented 4 years ago

Ah shoot, in the spirit of that bug i locally coded reminder skill like

def is_affirmative(self, utterance):
        affirmatives = self.translate_list['Affirmatives']

with Affirmatives.list holding {ja, sicher, bitte} -resp. other langs- in /dialog/lang/ (TypeError: 'method' object is not subscriptable)

(the former implementation would create a list in the method only holding english affirmatives)

could you point me in a direction where mycroft-core isn't (or wouldn't have to be) touched and would have a couple of options? Or should I (we) simply go the ask_yesno(dialog)-route?

(and maybe i'm wrong but i've seen several instances of this across skills where it's handled that way. Those should break, too)

forslund commented 4 years ago

I think in the error you have there is due to using [] where () should be used.

try

    def is_affirmative(self, utterance):
        affirmatives = self.translate_list('Affirmatives')