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

Fixed Currency to only Display Cents When Needed #602

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

Conversation

cheng-jess
Copy link
Contributor

Fixes #554 by updating base.py

Changes proposed in this pull request:

  • Updated to_currency() in base.py to only display cents when input has a decimal (or explicit cents)
  • e.g. 100 -> one hundred euro, 100.00 -> one hundred euro, zero cents
  • Changed tests to align with this fix

Status

  • READY
  • [] HOLD
  • [] WIP (Work-In-Progress)

How to verify this change

  • Run all tests in /tests
  • Try "num2words 100 -t currency" and "num2words 100.00 -t currency" in the command line

Additional notes

Not sure if this issue was a wanted bug fix or feature, but updated base.py to stop cents from being converted to words when not explicitly included in the number input (e.g. handling 100 vs 100.00 differently).

@cheng-jess
Copy link
Contributor Author

@mrodriguezg1991 Tagging for review per Mohammed's email. Thank you.

@mrodriguezg1991
Copy link
Contributor

@mrodriguezg1991 Tagging for review per Mohammed's email. Thank you.

Hello @cheng-jess , thanks for contributing
This PR appear to run on a loop on the test and never ends, can you review to make sure its ok ?

@bryananderson
Copy link
Contributor

@mrodriguezg1991 It looks like this is failing due to the Amharic cardinal test cases (28-33 in test_am.py)--these don't appear to be related to the cents issue.

@cheng-jess what's the purpose of these test cases?

I am also interested in getting this issue resolved. 🙂

@bryananderson
Copy link
Contributor

Oh it looks like it's related to @cheng-jess 's other PR here, probably these cases were meant to go there.

@bryananderson
Copy link
Contributor

I opened a new PR here which removes the erroneous test cases.

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.

Is there a way to stop cents being converted to words?
3 participants