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

Some swedish additions #2234

Merged
merged 4 commits into from Aug 2, 2019

Conversation

@c0r73x
Copy link
Contributor

commented Jul 25, 2019

Description

Added the functions nice_time_sv etc.

How to test

Tests are located in test_format_sv.py

Contributor license agreement signed?

CLA [ Yes ]

c0r73x added 2 commits Jul 25, 2019
@devs-mycroft

This comment has been minimized.

Copy link
Collaborator

commented Jul 25, 2019

Hello, @c0r73x, 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!

@forslund

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

Tack så mycket!

The time changes works very well, the pronounce number doesn't quite work 100% (but is a great improvement) If you want to check on it while waiting for the CLA (getting the mail sent out is a manual process if you don't get an e-mail let me know and I'll poke my colleague handling it) you can try

>>> pronounce_number(10000001, lang='sv-se')
'tioochmillionerer en'

>>> pronounce_number(1.1, lang='sv-se')
'en komma en noll'

I would probably not have pronounced the trailing "noll"

@c0r73x

This comment has been minimized.

Copy link
Contributor Author

commented Jul 27, 2019

I fixed the "Tiomiljarder en" but the "en komma en noll" is saying the same in all languages except English. An easy way to fix it is to set all languages to default places=1 to get the English behavior.

@c0r73x

This comment has been minimized.

Copy link
Contributor Author

commented Jul 28, 2019

I was looking at adding the duration parsing aswell. But it's very strange because it seems that there are alot of code that isn't language specific that are located in the file parse_en... Shouldn't alot of these functions and even the _ReplaceableNumber class be moved to the parse_common file to ease the duration parsing for other languages?

@devs-mycroft devs-mycroft added CLA: Yes and removed CLA: Needed labels Jul 28, 2019
@forslund

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

The "en komma en noll" isn't too big of an issue. The CLA seems to have arrived. I'm technically on vacation but I'll try to review it again and hopefully get it merged sometime during the week. (Going camping near Oxelöund before heading north :) )

Än en gång, stort tack för ditt bidrag.

@c0r73x

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

Inga problem ;) (trevlig semester, fel sida sverige dock :P)

I will look into moving some functions from the parse_en to parse_common to ease the translation, it's abit frustrating not to be able to say things like "set a timer for 3 minutes" in your own language.

@forslund

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

Ok, I've tested it again and looked over the code briefly and it looks good. I'll merge this now and look forward to the restructuring you're suggesting!

En än gång, stort tack för ditt bidrag.

@forslund forslund merged commit a140efe into MycroftAI:dev Aug 2, 2019
3 checks passed
3 checks passed
:-) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.04%) to 50.711%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.