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

Arabic translation enhancement: Hide unit count with dual forms. #222

Merged
merged 5 commits into from
May 25, 2024

Conversation

6km
Copy link
Contributor

@6km 6km commented May 14, 2024

The issue with is described in #221.
I have implemented a solution for it.

  • Adds a property to the Arabic humanizer called hideUnitInDualForm that I used to indicate that the language is Arabic.
    image

  • Adds a new option, which is arabic_hideUnitInDualForm, it works only with Arabic language.
    default: true.
    image

I could just check if the language equals ar, but I thought it would be better to add that hideUnitInDualForm property to the humanizer and options!

@EvanHahn
Copy link
Owner

Thank you! I will take a look soon.

Copy link
Owner

@EvanHahn EvanHahn left a comment

Choose a reason for hiding this comment

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

  • I don't know much about Arabic. Do you have a place I could learn more about these rules to make sure this is correct for all Arabic speakers?

  • Please run npm format to format the code.

  • It looks like tests are failing. Could you run npm test to make the tests pass?

humanize-duration.js Outdated Show resolved Hide resolved
humanize-duration.js Outdated Show resolved Hide resolved
@6km
Copy link
Contributor Author

6km commented May 23, 2024

@EvanHahn Thanks for your review! I'm working work on it!

  • I don't know much about Arabic. Do you have a place I could learn more about these rules to make sure this is correct for all Arabic speakers?

I didn't find many English resources talking about the dual nouns (Muthanna) in Arabic, but this can help I guess.
Number: Singular, Dual, Plural Noun in Arabic

A noun that ends with "ين" or "ان" is a dual noun; that's why we never put a number before it.

@6km 6km requested a review from EvanHahn May 23, 2024 17:13
@EvanHahn
Copy link
Owner

Thanks! I'll take another look soon.

@EvanHahn EvanHahn changed the base branch from main to merge-222 May 25, 2024 15:42
@EvanHahn EvanHahn merged commit e0a885c into EvanHahn:merge-222 May 25, 2024
1 check passed
EvanHahn added a commit that referenced this pull request May 25, 2024
See [#222](#222).

Co-Authored-By: Evan Hahn <[email protected]>
@EvanHahn
Copy link
Owner

This was released in [email protected]. Thanks for your contribution!

@6km
Copy link
Contributor Author

6km commented May 25, 2024

This was released in [email protected]. Thanks for your contribution!

You're welcome!

rasa pushed a commit to rasa/HumanizeDuration.js that referenced this pull request Sep 14, 2024
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