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

Changes for Basque language #2961

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open

Changes for Basque language #2961

wants to merge 13 commits into from

Conversation

e-gor
Copy link

@e-gor e-gor commented Jul 30, 2021

Changes for Basque language

Description

These changes, along with those sent to other repos, make Mycroft work in Basque language.

Made by Elhuyar, Talaios and Skura

Type of PR

Feature implementation

Dependencies

This PR relies on MycroftAI/lingua-franca#206

@pep8speaks
Copy link

pep8speaks commented Jul 30, 2021

Hello @e-gor! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-07-12 19:53:17 UTC

@devops-mycroft devops-mycroft added the CLA: Needed Need signed CLA from https://mycroft.ai/cla label Jul 30, 2021
@devops-mycroft
Copy link

Hello, @e-gor, thank you for helping with the Mycroft project! We welcome everyone
into the community and greatly appreciate your help as we work to build an AI
for Everyone.

To protect yourself, the project, and users of Mycroft technologies we require
a Contributor Licensing Agreement (CLA) before accepting any code
contribution. This agreement makes it crystal clear that along with your
code you are offering a license to use it within the confines of this project.
You retain ownership of the code, this is just a license.

Please visit https://mycroft.ai/cla to initiate this one-time signing. Thank
you!

@devops-mycroft
Copy link

devops-mycroft commented Jul 30, 2021

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@krisgesling
Copy link
Contributor

Hey Igor, this is awesome, thanks!

For the new TTS and STT modules, we've created a plugin system so that these don't need to be merged and maintained in Mycroft-core. This means we aren't continually expanding core, and makes it easier to get fixes into individual packages. It also means you don't need to wait for merge requests like this to get in. We'll be extracting the existing modules out and making them their own plugin packages as well, just haven't gotten to that yet.

Information on the plugin system is available here:
https://mycroft-ai.gitbook.io/docs/mycroft-technologies/mycroft-core/plugins

and there's a complete TTS example here:
https://github.com/forslund/mycroft-tts-plugin-gtts

So it will be easiest if this PR only contained translation additions. The nice_relative_time addition will need to be done with an update to Lingua Franca.

Cheers

@e-gor
Copy link
Author

e-gor commented Aug 4, 2021

So you mean I should take out the STT and TTS as modules? All right, but due to holidays, I do not think I will be able before September...

Regarding nice_relative_time, it is in the lingua-franca PR.

@devops-mycroft devops-mycroft added CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) and removed CLA: Needed Need signed CLA from https://mycroft.ai/cla labels Aug 4, 2021
@krisgesling
Copy link
Contributor

Yeah that's right, it's not sustainable for us to push everything into mycroft-core. The plugins help keep mycroft-core lighter and make it quicker for maintainers to distribute fixes.

Just for clarity I've added the PR to Lingua Franca as a dependency in this PR description.

@e-gor
Copy link
Author

e-gor commented Aug 31, 2021

Information on the plugin system is available here:
https://mycroft-ai.gitbook.io/docs/mycroft-technologies/mycroft-core/plugins
and there's a complete TTS example here:
https://github.com/forslund/mycroft-tts-plugin-gtts

Is this plugin from the same user a valid example of a STT plugin?

@krisgesling
Copy link
Contributor

Yeah, that looks right. There's also more available on PyPI if you want some other examples:
https://pypi.org/search/?q=mycroft+plugin+stt&o=

@e-gor
Copy link
Author

e-gor commented Sep 2, 2021

I have moved the Basque STT and TTS systems of Elhuyar to their own plugins:

They have been tested and work properly.

So now the PR only contains Basque translation additions and the nice_relative_time addition which is implemented in the lingua-franca PR.

@e-gor
Copy link
Author

e-gor commented Jun 20, 2022

Necessary changes in lingua-franca PRs done, @krisgesling. Once they are merged, this PR should have no conflict and could therefore be merged too.

@e-gor
Copy link
Author

e-gor commented Jul 8, 2022

lingua-franca now has the nice_relative_time function (PR has been merged), but integration test still fails saying it does not exist, I do not know why...

ASTRA053
ASTRA053 previously approved these changes Jul 8, 2022
@krisgesling
Copy link
Contributor

Hey Igor, we need to bump the Lingua Franca version to 0.4.3 in requirements/requirements.txt

I tried to push this to your branch but understandably don't have write access.

That should make the new function available, along with the rest of the Basque support.

@e-gor
Copy link
Author

e-gor commented Jul 12, 2022

Sorry, my mistake (again), was not aware of that...

Did the required change, but there are still errors in the integration tests (though I do not think they are due to our changes, they are in Wikipedia skill, apparently).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants