-
Notifications
You must be signed in to change notification settings - Fork 506
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
add kannada language support #243
Conversation
@erozqba tests are failing for |
@akki2825 can you update your branch? |
@kkonieczny thanks for the reply. I did update, its passing on py36 env but failing on others. Not sure what I'm missing missing. |
num2words/__init__.py
Outdated
@@ -101,8 +102,6 @@ def num2words(number, ordinal=False, lang='en', to='cardinal', **kwargs): | |||
if lang not in CONVERTER_CLASSES: | |||
raise NotImplementedError() | |||
converter = CONVERTER_CLASSES[lang] | |||
if isinstance(number, str): | |||
number = converter.str_to_number(number) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @akki2825 you are missing this section. With this conditional, the tests pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @lepisma
Pull Request Test Coverage Report for Build 698
💛 - Coveralls |
ebb04b2
to
8e2677a
Compare
I am very much interested in this. How is the status of this pull request? |
@sarahberanek I don't know kannada but I believe there were/are certain inaccuracies in few numbers or number ranges. We use a fork (not PRed here) which probably has those fixed. @akki2825 can you check this fork's code and say something about the changes? |
@lepisma ah that is interesting! Thank you for the info. Any way that we can fix the bugs here and merge a correct version into the official master? |
@lepisma thanks for the info. I have missed a condition in number ranges. @sarahberanek, I'll make the necessary changes and get back by tomorrow. |
Super, thanks! |
Because there might be a new release this week of num2words, can we include this into the master? :-) Highly appreciated! |
@akki2825 Thanks a lot for this contribution! I have tried to resolve a conflict on your PR, but my changes were not correct. Please check Travis and Coveralls responses. Could you also add more tests? You could check on Travis the missed lines. I will be doing a new release this weekend, it will be great to include this on the release. |
@akki2825 I will do a release tomorrow, do you think you will have time to finish this before the release so we can make @sarahberanek happy? |
@akki2825 thanks a lot! We are almost there just missing a couple of tests to cover these lines https://coveralls.io/builds/23334032/source?filename=num2words%2Flang_KN.py |
Hey, I was a bit occupied. Apologies for the delay. @erozqba. you could merge it now. |
@akki2825 I forgot to mention before, could you update the README.md to add this new language as part of your PR? |
@erozqba done. |
|
Fixes # by...
Changes proposed in this pull request:
Adds Kannada language support.
Status