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

Fixing TAMIL NUMBER TEN replacement #10

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

amrzv
Copy link
Contributor

@amrzv amrzv commented May 1, 2024

Hello.

  1. Since Tamil table is the only one with 11 digits, added if statement for (TAMIL NUMBER TEN).
  2. Added return '' in case empty input.
  3. Bumps the version of the package.

@@ -29,9 +31,9 @@ def num_to_word(num, lang, separator=", ", combiner=" "):
for language in SUPPORTED_LANGUAGES:
for num_index in range(10):
num = num.replace(DIGITS_LANG_MAPPING[language][num_index], DIGITS_LANG_MAPPING["en"][num_index])
if language == 'ta':
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use exception.py file for the language specific exception

@@ -29,9 +31,7 @@ def num_to_word(num, lang, separator=", ", combiner=" "):
for language in SUPPORTED_LANGUAGES:
for num_index in range(10):
num = num.replace(DIGITS_LANG_MAPPING[language][num_index], DIGITS_LANG_MAPPING["en"][num_index])

# Assert that input contains only integer number
assert num, "Input string is empty"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the assertation has been removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above I added

    if not num:
        return ''

to return empty string if provided.


# Assert that input contains only integer number
assert num, "Input string is empty"
num = language_specific_exception(num, lang, combiner)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function has been planned to handle exception after the conversation,
using this function does not feel right, we can have two function for the exception,

  • one before the converstion (what you are trying to do)
  • one after the conversation (what the function language_specific_exception is currently doing)

@@ -59,4 +59,6 @@ def occurs_at_end(piece):
for expt in exception_dict:
if occurs_at_end(expt):
words = words.replace(expt, exception_dict[expt])
elif lang == "ta":
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point but I would keep things simple.

There are two school of though here.
In modern usage, the Tamil language predominantly uses the Hindu-Arabic numeral system, where "௧௦" (the combination of 1 and 0) is used to represent the number 10. The traditional Tamil numeral "௰" for 10 is less commonly used today and is primarily found in historical texts, inscriptions, and certain cultural or ceremonial contexts.

So, "௧௦" is more common than "௰" in contemporary usage.
(I might be wrong with my limited tamil knowledge, please attach some source if you think otherwise)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an opinion there. I thought the digit symbol ten should be handled anyway since it exists.

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

Successfully merging this pull request may close these issues.

2 participants