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 issues with wrong text for currency in Polish language #58

Merged
merged 7 commits into from
Feb 27, 2017

Conversation

kszys
Copy link
Contributor

@kszys kszys commented Feb 25, 2017

When the amount to be converted to text using to_currency function in Polish language was ending with a number of decimals being a whole 10 - e.g., 11.30, 123.40, 34234.80, etc. the resulting text was saying respectively three, four, eight (in Polish) rather than thirty, forty, eighty. This was because the same conversion was used for regular numbers as for cents, which is incorrect. This patch fixes it for Polish language.

@erozqba
Copy link
Collaborator

erozqba commented Feb 27, 2017

Hi @kszys
Thanks a lot for your contribution, could you also include a test in you PR to avoid future regressions? Right now we don't have any test for Polish and will be great to start testing it.

@kszys
Copy link
Contributor Author

kszys commented Feb 27, 2017

I added the unittests, but the checks seem to fail... The tests worked fine, when I tested locally - not sure what I need to do now...

@erozqba
Copy link
Collaborator

erozqba commented Feb 27, 2017

Hi @kszys
Thanks a lot for the unit tests, looking at it, the problems is that there is not python 3 compatible. All tests pass for python 2.7, but we are running tests in 4 different python versions 2.7, 3.4, 3.5, 3.6.

For more information, you can look at https://travis-ci.org/savoirfairelinux/num2words/jobs/205850020
The fails is in

test_currency

Thanks in advance if you can look at it and fix it.
I will try to look at it later when I find some time.

@kszys
Copy link
Contributor Author

kszys commented Feb 27, 2017

Ok. It seems I have not tested with python 3 :)
I will take a look later today.

@kszys
Copy link
Contributor Author

kszys commented Feb 27, 2017

This should do it :)

right = n % 100
else:
n = str(n).replace(',', '.')
if '.' in n:
left, right = n.split('.')
if len(right)==1:
right = right+'0'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add whitespace around the operators? So we keep respecting PEP8?
if len(right) == 1: and right = right + '0'

@erozqba
Copy link
Collaborator

erozqba commented Feb 27, 2017

Hi @kszys thanks for your work! We will change tox in the future so it will automatically check for PEP8

@erozqba erozqba merged commit 710a3b6 into savoirfairelinux:master Feb 27, 2017
@kszys
Copy link
Contributor Author

kszys commented Feb 27, 2017

So when is this going to be pushed in a new version of the module that I can install with pip? I need it working on a few systems ;)

@erozqba
Copy link
Collaborator

erozqba commented Feb 27, 2017

Yes, @hsoft is usually in charge of the releases, he will probably make a new one when he finds some free time. 👍

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