3
\$\begingroup\$

The following code converts a number into Indian numbering system format.

I know locale can also help in getting similar format but I wanted something that can work on any system because not all locales are present in all OSs.

The code was written for Python 3.5 hence no variable annotations.

from decimal import Decimal
from itertools import islice, zip_longest
from typing import Any, Iterable, Iterator, Sequence, Tuple, TypeVar, Union  # noqa: ignore=F401


_T = TypeVar('_T')


def grouper(iterable: Iterable[_T], n: int, fillvalue: Any = None) -> Iterator[Tuple[_T, ...]]:
    "Collect data into fixed-length chunks or blocks"
    # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx
    args = [iter(iterable)] * n
    return zip_longest(fillvalue=fillvalue, *args)


def indian_numbering_format(
    amount: Union[Decimal, int, str], places: int = 2,
    curr: str = '', pos: str = '', neg: str = '-'
) -> str:
    """
    Formats a number and returns it in Indian numbering format
    https://en.wikipedia.org/wiki/Indian_numbering_system

    places:  optional number of places after the decimal point
    curr:    optional currency symbol before the sign
    pos:     optional sign for positive numbers: '+' or blank
    neg:     optional sign for negative numbers: '-' or blank

    Note that if the fractional part only contains 0 it will be
    dropped.
    """
    q = Decimal(10) ** -places  # type: Decimal
    amount = Decimal(amount).quantize(q)
    sign, digits, exponent = amount.as_tuple()
    integer_part, fractional_part = (), ()  # type: Sequence[int], Sequence[int]
    exponent = exponent or len(digits)
    integer_part = digits[:exponent]
    # If fractional part only contains 0s then ignore it.
    if set(digits[exponent:]) != {0}:
        fractional_part = digits[exponent:]
    integer_part_digits = reversed(''.join(map(str, integer_part)))
    thousand = ''.join(islice(integer_part_digits, 3))[::-1]
    hundreds = ','.join(map(''.join, grouper(integer_part_digits, 2, fillvalue='')))[::-1]
    sep = ',' if hundreds else ''
    fractional_part_digits = '.' + ''.join([str(d) for d in fractional_part]) if fractional_part else ''
    return '{sign}{curr}{hundreds}{sep}{thousand}{fractional_part}'.format(
        sign=neg if sign else pos,
        curr=curr,
        thousand=thousand,
        sep=sep,
        hundreds=hundreds,
        fractional_part=fractional_part_digits
    )

I was wondering if the code can be refactored in any way to make it much more simpler.

\$\endgroup\$
2
\$\begingroup\$

I don't think I'd get so fancy with the grouper; the data is small, a constant table will do:

comma_after_digit = set(3, 5, 7, 9, 11, 13, 15, 17)

result = '.' + ''.join(map(str, fractional_part))
for i, d in enumerate(integer_part_digits):
    if i in comma_after_digit:
        result = ',' + result
    result = d + result
result = {0:pos, 1:neg}[sign] + result
return result

Also:

  1. Try and be a bit more consistent: you use reversed() to reverse a string on one line, and then the next you use [::-1]. You also use map(str, digits) in one place and [str(x) for x in digits] another! They do the same thing, but it's nice to the reader to pick one idiom and stick with it.

  2. thousand = ''.join(islice(integer_part_digits, 3))[::-1] can be simplified into thousand = integer_part_digits[-4::-1]

  3. Using .format() is overkill for concatenating strings. + works just fine. If you want to use it, you should 1) factor out constant strings (like the decimal point) and 2) use the args as your temporary variables, like:

    return '{sign}{curr}{hundreds}{sep}{thousand}.{fractional_part}'.format(
        sign=neg if sign else pos,
        curr=curr,
        thousand=integer_part_digits[-4::-1],
        sep=',' if exponent > 3 else '',
        hundreds=','.join(map(''.join, grouper(integer_part_digits, 2, fillvalue='')))[::-1],
        fractional_part=''.join(map(str,fractional_part)) if fractional_part else ''
    )
    
  4. According to the wikipedia page you cite, there's no comma after the 1 in 10**19, so your code is wrong :) ... which leads inevitably to:

  5. Needs tests!

\$\endgroup\$
  • \$\begingroup\$ Good answer. For 2, I'd suggest for OP to stick to list comprehension. For 4, refer to this SO post, since performance varies when concatenating vs substituting strings. \$\endgroup\$ – omgimanerd Jul 15 '17 at 5:45
  • \$\begingroup\$ 5. I think 10**19 defined on Wiki page is incorrect because LC_MONETARY for en_IN says the grouping structure is 3;2. I already have decent number of tests :-). 2. Agreed with the inconsistency part, I had to introduce the map inconsistency due to some issue with Mypy, but I guess I should add a comment about it. 4. The idea behind using format string there is that we will replace it with a f-string when we move to Python 3.6, it would be much more compact after that. \$\endgroup\$ – Ashwini Chaudhary Jul 15 '17 at 6:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.