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

[FIX] tools: patch fixed Arabic class from newer version of num2words. #156286

Closed
wants to merge 1 commit into from

Conversation

MohammedBasioni
Copy link
Contributor

@MohammedBasioni MohammedBasioni commented Mar 4, 2024

patch fixed Arabic class from newer version of num2words package.

Description of the issue/feature this PR addresses

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. Also, it was noticed that this behaviour happens in all multiple of 1000s. It appends 'one' to million and billion, etc.

Current behaviour before PR

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

Desired behaviour after PR is merged:

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

@robodoo
Copy link
Contributor

robodoo commented Mar 4, 2024

Pull request status dashboard.

@C3POdoo C3POdoo requested a review from a team March 4, 2024 12:23
@oomsveta oomsveta marked this pull request as draft March 4, 2024 12:41
@C3POdoo C3POdoo added the RD research & development, internal work label Mar 4, 2024
@MohammedBasioni MohammedBasioni changed the title [FIX] monkey patch [FIX] change the patching to patch the whole Arabic class from newer version of num2words where the problem is fixed. Mar 4, 2024
@MohammedBasioni MohammedBasioni changed the title [FIX] change the patching to patch the whole Arabic class from newer version of num2words where the problem is fixed. [FIX] patch the whole Arabic class from newer version of num2words where the problem is fixed. Mar 4, 2024
@MohammedBasioni MohammedBasioni changed the title [FIX] patch the whole Arabic class from newer version of num2words where the problem is fixed. [FIX] tools: patch fixed Arabic class from newer version of num2words package. Mar 5, 2024
@MohammedBasioni MohammedBasioni changed the title [FIX] tools: patch fixed Arabic class from newer version of num2words package. [FIX] tools: patch fixed Arabic class from newer version of num2words. Mar 5, 2024
odoo/tools/__init__.py Outdated Show resolved Hide resolved
odoo/tools/num2words_monkeypatches.py Outdated Show resolved Hide resolved
odoo/tools/num2words_monkeypatches.py Outdated Show resolved Hide resolved
odoo/tools/num2words_monkeypatches.py Outdated Show resolved Hide resolved
@MohammedBasioni MohammedBasioni force-pushed the 17.0-fix-num2words-basm branch 2 times, most recently from 4484357 to 6901edd Compare April 24, 2024 12:42
@MohammedBasioni MohammedBasioni changed the base branch from 17.0 to 16.0 April 25, 2024 02:20
odoo/tools/_monkeypatches.py Outdated Show resolved Hide resolved
odoo/tools/_monkeypatches.py Outdated Show resolved Hide resolved
odoo/tools/num2words_patch.py Outdated Show resolved Hide resolved
@MohammedBasioni MohammedBasioni force-pushed the 17.0-fix-num2words-basm branch 3 times, most recently from 46a0429 to 8362d0e Compare April 29, 2024 09:50
@oomsveta oomsveta marked this pull request as ready for review April 29, 2024 09:52
@C3POdoo C3POdoo requested review from a team, xmo-odoo and HydrionBurst and removed request for a team April 29, 2024 10:10
@oomsveta
Copy link
Contributor

robodoo r+

@fw-bot
Copy link
Contributor

fw-bot commented May 4, 2024

@MohammedBasioni @oomsveta this pull request has forward-port PRs awaiting action (not merged or closed):

2 similar comments
@fw-bot
Copy link
Contributor

fw-bot commented May 5, 2024

@MohammedBasioni @oomsveta this pull request has forward-port PRs awaiting action (not merged or closed):

@fw-bot
Copy link
Contributor

fw-bot commented May 6, 2024

@MohammedBasioni @oomsveta this pull request has forward-port PRs awaiting action (not merged or closed):

@lse-odoo
Copy link
Contributor

@MohammedBasioni
Hey :) Your PR unfortunately create a crash with the IoT box.

The IoT box uses odoo's community code for its core features with it's own requirement.
In it's latest version, the Odoo code that the IoT box uses does not import the num2words library (as we don't need it).

I will see with the IoT team if we can solve that on our end without modifying your file, but likely not (as we need to build new IoT OS images for library changes and reflash SD cards). Just be aware of this in case other people come to you on the matter and I'll come back to you if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants