change 'یکم' to 'اول'

[webneshin]

In Persian, we often use "اول" instead of "یکم". For example, "گل اول رو زد" or "اولین قسمت منتشر شد"

[mrunderline]

PR #13 Review Comment: Changing 'یکم' to 'اول'

Hi @webneshin 👋

Thank you for this PR and improving the library. Changing 'یکم' to 'اول' for the number 1 in ordinal mode is a great idea since "اول" is more commonly used in Persian.

✅ Strengths

1. Changes in ordinal_suffix.py: Adding support for 'اول' and 'اولین' in the add() and remove() functions is done correctly.
2. Adding mapping in strings.py: Adding "اول": "یک" to TYPO_LIST for reverse conversion support is good.
3. Comprehensive tests: The added tests provide good coverage for various cases.

⚠️ Issues Identified

Issue 1: number == 1 check without considering the ordinal parameter

In your changes to persian_tools/digits/__init__.py, this code was added:

if number == 0:
    return 'صفر'
elif number == 1:
    return 'اول'

Problem: This code always returns "اول" even when ordinal=False! This causes convert_to_word(1, ordinal=False) to return "اول" instead of "یک", which is incorrect.

Suggested Solution:

The best approach is to perform this check after complete processing and before adding the ordinal suffix:

if number == 0:
    return 'صفر'

is_negative = number < 0
number = abs(number)

# ... rest of the processing (lines 69-91) ...

words = words.strip()

if is_negative:
    words = 'منفی ' + words

if ordinal:
    # Special case: 1 should be "اول" not "یکم"
    if words == 'یک' or words == 'منفی یک':
        words = words.replace('یک', 'اول')
    else:
        words = add_ordinal_suffix(words)

return words

Important Note: Actually, the changes you made in ordinal_suffix.py (ordinal_suffix.add('یک') → 'اول') are sufficient! When convert_to_word(1, ordinal=True) is called, it first generates "یک" and then calls add_ordinal_suffix('یک'), which with your changes returns "اول". So there's no need for an additional check in digits/__init__.py and you can remove it.

Issue 2: Lack of support for negative numbers

If number == -1 and ordinal=True, it should return "منفی اول". The current changes don't account for this case.

Issue 3: Processing order

The number == 1 check should be performed after abs(number) to also cover negative numbers.

🔍 Review of Existing Tests

Existing tests that might be affected:

• test_convert_to_word(): If convert_to_word(1) is called without ordinal=True, it should return "یک" not "اول"

📝 Suggestions

1. Fix the number == 1 check: Should only apply when ordinal=True
2. Support for negative numbers: Should also support "منفی اول"
3. Additional test: Add a test for convert_to_word(1, ordinal=False) which should return "یک"
4. Test for negative numbers: Add a test for convert_to_word(-1, ordinal=True) which should return "منفی اول"

✨ Conclusion

The PR idea is excellent and the changes in ordinal_suffix.py and strings.py are correct, but needs correction in digits/__init__.py to only apply in ordinal mode.

Thank you for this improvement! 🙏