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.

Sign up for GitHub

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

Jump to bottom

Fix several issues with num2words in Arabic #512

Merged
merged 17 commits into from
Apr 11, 2023
Merged

Fix several issues with num2words in Arabic #512

merged 17 commits into from
Apr 11, 2023

Conversation

Jeronymous
Copy link
Contributor

@Jeronymous Jeronymous commented Mar 29, 2023 •
edited

There are several issues with Arabic language: https://github.com/savoirfairelinux/num2words/issues?q=is%3Aissue+is%3Aopen+arabic

Some previous work was never merged (and got outdated): #404

This PR fixes:

also a bug on French:

This PR does not fix (yet?)

@Jeronymous Jeronymous closed this Mar 31, 2023
@hedhoud
solve ar:
283d3ca 
-chahnge the max number to 10 ** 51 -1
-solve the et in 262 and others
-add some tests
@hedhoud
solving ar issues:
4863fb1 
 - change maxval to  10 ** 51.
 - add comments to abs & to_str functions.
 - change a list of integer_value by a pow condition
@hedhoud
solve the one problem
667723a
@hedhoud
solve the 10000 currency problem
ff9385f
@Jeronymous Jeronymous reopened this Apr 5, 2023
@Jeronymous
Copy link
Contributor Author

@mrodriguezg1991 Is there somebody who could review this?

@mrodriguezg1991
Copy link
Contributor

@mrodriguezg1991 Is there somebody who could review this?

Hello, thanks for your contribution, can you update your code to pass flake8 validation ??

@Jeronymous
Copy link
Contributor Author

Hello, thanks for your contribution, can you update your code to pass flake8 validation ??

@mrodriguezg1991 Thanks for spotting. Code formatting issues are fixed now.

@coveralls
Copy link

coveralls commented Apr 11, 2023 •
edited

Coverage Status

Coverage: 97.389% (+0.2%) from 97.177% when pulling 3e7d607 on linto-ai:bugfix/arabic into d098627 on savoirfairelinux:master.

@mrodriguezg1991
Copy link
Contributor

@Jeronymous can you pull the master branch in your PR?, because it does not let me merge because the PR is behind

@Jeronymous
Copy link
Contributor Author

@mrodriguezg1991 Sure. Merged

@mrodriguezg1991 mrodriguezg1991 merged commit be6b2df into savoirfairelinux:master Apr 11, 2023
6 checks passed
@Jeronymous
Copy link
Contributor Author

Thanks for merging @mrodriguezg1991

Note that these issues could be closed (we forgot to mention them in commit messages, and I don't have permissions to close them):

oomsveta added a commit to odoo-dev/odoo that referenced this pull request Feb 27, 2024
@oomsveta
[FIX] upgrade num2words library to 0.5.13
37e2c19 
As of version 0.5.8, the num2words library does not correctly convert
some numbers to words in Arabic. It erroneously appends the Arabic
equivalent of "one" to the translation of 1000 (one thousand), when it
must be implicit.

Expected output:

'ألف و مئتان و أربعة و ثلاثون  , ست و خمسون'

0.5.9:
>>> from num2words import num2words
>>> num2words(1234.56, lang="ar")
'واحد ألف  و مئتان و أربعة و ثلاثون  , ست و خمسون'

This bug has been fixed in version 0.5.13 of the library:

0.5.13:
>>> from num2words import num2words
>>> num2words(1234.56, lang="ar")
'ألف و مئتان و أربعة و ثلاثون  , ست و خمسون'

This commit updates the requirements.txt to use the version of the
library that produces the correct output.

opw-3756778

References:
+ savoirfairelinux/num2words#512
@oomsveta oomsveta mentioned this pull request Feb 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrodriguezg1991 mrodriguezg1991 mrodriguezg1991 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Jeronymous @mrodriguezg1991 @coveralls @hedhoud