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

Add negative sign after currency names (#937) #938

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michaelfm1211
Copy link

This PR changes babel.numbers.parse_pattern to move the negative sign to the end of the prefix if the format string includes a placeholder for a currency name. It updates some tests.

@DenverCoder1
Copy link
Contributor

I don't know about all locales, but at least in the US, I find it much more common to see "-$123.46M" than "$-123.46M".

Is there a source for this format being preferred?

@michaelfm1211
Copy link
Author

This PR is to address this issue that I opened (sorry if that wasn't as clear as it should've been). The rationale behind it is that "-$123.46M" is normal, but "-CHF 123.46M" isn't. With the change, it would fix the latter to "CHF -123.46M".

@DenverCoder1
Copy link
Contributor

DenverCoder1 commented Dec 27, 2022

It seems to me that with this change it outputs 'CHF-12,345.00' when it should probably have a space. And it would best to make it so that it does not affect the outputs which are already correct such as the '-$123.46M' case.

>>> format_currency(-12345, 'CHF', locale="en_US")
'CHF-12,345.00'

Maybe it could determine the position based on the length of the currency symbol or whether it contains only letters?

It would probably be a good idea to check if the unicode.org / CLDR docs say anything about it. It doesn't seem like there's any help here although it does say 'negative currency amounts might typically be displayed as something like “-$3.27”'

@DenverCoder1
Copy link
Contributor

There may be other solutions, but I might suggest to put this back to how it was

-        neg_prefix = f"-{pos_prefix}" if '¤' not in pattern else f'{pos_prefix}-'
+        neg_prefix = f"-{pos_prefix}"

And instead change where it replaces the currency symbols to guess the minus sign location based on the currency symbol. For example:

# if the currency symbol is the same as the currency code
# and the value is negative, move the currency symbol
# to be before the number instead of before the currency code
if currency_symbol == currency_code and is_negative:
    retval = retval.replace(r'-¤', r'¤ -')
         if u'¤' in retval:
-            retval = retval.replace(u'¤¤¤',
-                                    get_currency_name(currency, value, locale))
-            retval = retval.replace(u'¤¤', currency.upper())
-            retval = retval.replace(u'¤', get_currency_symbol(currency, locale))
+            currency_name = get_currency_name(currency, value, locale)
+            currency_code = currency.upper()
+            currency_symbol = get_currency_symbol(currency, locale)
+            retval = retval.replace(u'¤¤¤', currency_name)
+            retval = retval.replace(u'¤¤', currency_code)
+            # if the currency symbol is the same as the currency code
+            # and the value is negative, move the currency symbol
+            # to be before the number instead of before the currency code
+            if currency_symbol == currency_code and is_negative:
+                retval = retval.replace(r'-¤', r'¤ -')
+            retval = retval.replace(u'¤', currency_symbol)

This allows changing the behavior only in the problematic cases, being the ones where the symbol (¤) is the three letter abbreviation and not a currency sign like "$".

Feel free to share your thoughts on this.

@michaelfm1211
Copy link
Author

michaelfm1211 commented Dec 27, 2022

I'm not sure if that change would be effective in all cases, especially when using the full currency names. For example, with your suggested change:

>>> format_currency(-100.50, 'CHF', '¤¤ #,##0.00', locale='de_CH')
'-CHF 100.50'
>>> format_currency(-100.50, 'CHF', '¤¤¤  #,##0.00', locale='de_CH')
'-Schweizer Franken  100.50'

However, the code from the original PR prevents the odd negative sign before letters:

>>> format_currency(-100.50, 'CHF', '¤¤ #,##0.00', locale='de_CH')
'CHF -100.50'
>>> format_currency(-100.50, 'CHF', '¤¤¤  #,##0.00', locale='de_CH')
'Schweizer Franken  -100.50'

Let me know if you observed any other results, or if I am at all wrong about this

I should also note that this PR doesn't affect the default format because the parse_pattern() is only called when a custom format is not provided to format_currency(). Also, custom formats with a negative subpattern are not affected either. So when using the default format this is the behavior:

>>> format_currency(-100.50, 'CHF', locale='de_CH')
'CHF-100.50'
>>> format_currency(-100.50, 'USD', locale='en_US')
'$-100.50'

@DenverCoder1
Copy link
Contributor

At least in my opinion, in order for this to be a positive change, it needs to be able to differentiate with complete accuracy between a currency label being text vs. being a symbol since the default behavior for format_currency(-100.50, 'USD', locale='en_US') should definitely stay as -$100.50 to avoid making an unexpected change to projects that rely on that.

As it would need to work with all currencies and all locales, it seems like it may not be so simple to differentiate here, in which case I think leaving it with format_currency(-100.50, 'CHF', locale='de_CH') == '-CHF100.50' may be the most practical approach. There is always the format='¤¤ #,##0.00;¤¤ -#,##0.00' workaround from StackOverflow that can be used as a workaround after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants