Number to words in Python 3

Question

Here is a sample program I have written to convert numbers to words:

ONES = [None, 'one', 'two', 'three', 'four',
        'five', 'six', 'seven', 'eight', 'nine']

# Empty string for zero will be replaced in main function

TEENS = ['ten', 'eleven', 'twelve', 'thirteen', 'fourteen',
         'fifteen', 'sixteen', 'seventeen', 'eighteen', 'nineteen']

TENS_PREFIXES = ['_', '_', 'twen', 'thir', 'for',
                 'fif', 'six', 'seven', 'eigh', 'nine']

# Tens places words without "-ty". ex. twenty > twen, thirty > thir

POWERS_OF_1000 = ['', 'thousand', 'million', 'billion', 'trillion',
                  'quadrillion', 'quintillion', 'sextillion',
                  'septillion', 'octillion']


def to_base_1000(number):
    # To split a number into chunks of three digits
    # ie. (fifty three thousand   two hundred one)

    digits = []
    while number:
        number, digit = divmod(number, 1000)
        digits.append(digit)

    return digits


def to_hundreds_tens_ones(number):

    assert(0 <= number < 1000)

    return number // 100, number // 10 % 10, number % 10


def two_digit_to_words(tens, ones):

    if tens == 0:
        return ONES[ones]

    elif tens == 1:
        return TEENS[ones]

    elif ones == 0:
        return '{}ty'.format(TENS_PREFIXES[tens])
        # This won't index to the underscores (positions 0 and 1)
        # because of the first two ifs

    else:
        return '{}ty {}'.format(TENS_PREFIXES[tens], ONES[ones])


def three_digit_to_words(hundreds, tens, ones):

    last_two_digits = two_digit_to_words(tens, ones)
    if hundreds == 0:
        return last_two_digits

    else:
        if last_two_digits is None:
            return '{} hundred'.format(ONES[hundreds])

        else:
            return '{} hundred {}'.format(ONES[hundreds], last_two_digits)


def chunk_to_words(chunk, position):

    multiplier = POWERS_OF_1000[position]
    # Thousand, million, etc.

    if multiplier:
        multiplier = ' ' + multiplier

    # Add space if it exists so that it's not "onemillion" "twothousand" etc.

    hundreds, tens, ones = to_hundreds_tens_ones(chunk)

    return three_digit_to_words(hundreds, tens, ones) + multiplier


def number_to_words(number):

    if number == 0:
        return 'zero'

    chunks = to_base_1000(number)

    chunk_words = []

    for position, chunk in enumerate(chunks):
        chunk_words.append(chunk_to_words(chunk, position))

    return ' '.join(chunk_words[::-1])

    # Reverse because to_base_1000 returns chunks from smallest to largest

I would like some advice on improving my programming style and my commenting.

Answer 1

Bug

I generally like your logic. You have a bug, though:

>>> number_to_words(1_000_001)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "cr175527.py", line 94, in number_to_words
    chunk_words.append(chunk_to_words(chunk, position))
  File "cr175527.py", line 81, in chunk_to_words
    return three_digit_to_words(hundreds, tens, ones) + multiplier
TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'

The problem is that for middle chunk of three digits, three_digit_to_words(0, 0, 0) evaluates to None, which isn't properly handled.

Comments and style

I find it annoying that your comments often appear after the corresponding code. It is more conventional to either put the comment before the code, or at the end of the same line.

The most important kind of comment is the docstring, and you didn't write any. (Your comment on to_base_1000() was almost like a docstring.) Fortunately, your function names were clear enough to indicate their purpose. Still, one feature of docstrings that you are missing out on is the ability to embed doctests, which would be extremely helpful for this kind of code. For each function, a simple example, a complex example, and some corner cases would clarify exactly how it behaves.

I would change your brace style for the definitions of ONES, TEENS, etc. so that they all have the same indentation. Also, if you write ONES = [None, 'one', …], then you should also be consistent and write TENS_PREFIXES = [None, None, 'twen', …].

Logic and organization

I am slightly annoyed by your

TENS_PREFIXES = ['_', '_', 'twen', 'thir', 'for',
                 'fif', 'six', 'seven', 'eigh', 'nine']

# Tens places words without "-ty". ex. twenty > twen, thirty > thir

There is no situation in which you would ever output "twen" rather than "twenty", so why skimp on those letters, when you could have written 'twenty' and avoided the need for that apologetic comment?

If you use "chunk" as your terminology for a block of three digits, consider using that name instead of to_base_1000(). Then, your numbers_to_words() function would be a composition of chunk_to_words() and number_to_chunks(), which I find pleasing.

In my opinion, you would be better off rolling the helper functions to_hundreds_tens_ones(), two_digit_to_words(), and three_digit_to_words() into chunk_to_words(). The code would be more readable with less jumping around.

A lot of the conditions exist to ensure that exactly one space appears between words. For example:

if last_two_digits is None:
    return '{} hundred'.format(ONES[hundreds])

else:
    return '{} hundred {}'.format(ONES[hundreds], last_two_digits)

I would prefer to yield each word as a generator, then let ' '.join(…) take care of inserting the spaces.

Suggested solution

ONES = [
    None, 'one', 'two', 'three', 'four',
    'five', 'six', 'seven', 'eight', 'nine'
]

TEENS = [
    'ten', 'eleven', 'twelve', 'thirteen', 'fourteen',
    'fifteen', 'sixteen', 'seventeen', 'eighteen', 'nineteen'
]

TENS = [
    None, None, 'twenty', 'thirty', 'forty',
    'fifty', 'sixty', 'seventy', 'eighty', 'ninety'
]

POWERS_OF_1000 = [
    None, 'thousand', 'million', 'billion', 'trillion',
    'quadrillion', 'quintillion', 'sextillion', 'septillion', 'octillion'
]


def number_to_chunks(number):
    """
    Split an integer into little-endian base-1000 chunks.

    >>> list(number_to_chunks(1234))
    [234, 1]
    >>> list(number_to_chunks(31415000))
    [0, 415, 31]
    >>> list(number_to_chunks(0))
    []
    """
    while number:
        number, n = divmod(number, 1000)
        yield n

def chunk_to_words(chunk, scale):
    """
    Generate English words given a chunk (an integer less than 1000) and its
    scale (power of 1000).

    >>> list(chunk_to_words(31, 1))
    ['thirty', 'one', 'thousand']
    >>> list(chunk_to_words(50, 2))
    ['fifty', 'million']
    >>> list(chunk_to_words(0, 0))
    []
    >>> list(chunk_to_words(12, 3))
    ['twelve', 'billion']
    """
    assert 0 <= chunk < 1000
    hundreds, tens, ones = chunk // 100, chunk // 10 % 10, chunk % 10

    if hundreds:
        yield ONES[hundreds]
        yield 'hundred'

    if tens == 1:
        yield TEENS[ones]
    else:
        if tens:
            yield TENS[tens]
        if ones:
            yield ONES[ones]

    if chunk and scale:
        yield POWERS_OF_1000[scale]

def number_to_words(number):
    """
    Convert a nonnegative integer less than 10**30 to an English phrase.

    >>> number_to_words(103)
    'one hundred three'
    >>> number_to_words(0)
    'zero'
    >>> number_to_words(1_000_001)
    'one million one'
    >>> number_to_words(31_415_926_535)
    'thirty one billion four hundred fifteen million nine hundred twenty six thousand five hundred thirty five'
    """
    chunks = [
        ' '.join(chunk_to_words(chunk, i))
        for i, chunk in enumerate(number_to_chunks(number))
    ]
    return ' '.join(c for c in chunks[::-1] if c) or 'zero'

Answer 2

I would like some advice on improving my programming style and my commenting.

So let it be written; so let it be done!

Some Comments on Commenting

On your commenting style: I find it annoying and inadequate.

Your very first comment was (1) after the affected object; and (2) did not explain what I wanted to know. Subsequent comments were also postfix, which is irritating, and frequently were better written as docstrings:

Consider this:

ONES = [None, 'one', 'two', 'three', 'four',
        'five', 'six', 'seven', 'eight', 'nine']

# Empty string for zero will be replaced in main function

First, there is no empty string for zero. So I assume you mean "None will be replaced with an empty string". So why not store it in there now? Next, there is no main function, so I assume you mean number_to_words, which I have to find by manually searching. Except there is no special case there, save for the if number == 0: return 'zero' which does address the special case but doesn't handle the whole "why is ONES[0] not equal to ''" question in my mind.

Of course, there is some handling for that, but it's in the two_digit_to_words function, and it's not a replacement, but rather it's handled in code flow using an if statement.

So 2 lines in to your program, and your comments are lies.

This is why comments are generally frowned upon. You can't count on them to be true, or stay true - they don't get unit tested, and they don't get compiled.

I would suggest that you simply store '' into ONES[0] and delete the comment below it.

Another example:

Consider this:

def to_base_1000(number):
    # To split a number into chunks of three digits
    # ie. (fifty three thousand   two hundred one)

This comment should not be a comment. If you're going to explain the purpose of the function, use a docstring.

And another one:

Consider this:

elif ones == 0:
    return '{}ty'.format(TENS_PREFIXES[tens])
    # This won't index to the underscores (positions 0 and 1)
    # because of the first two ifs

Here we have a comment after the code in question, which is annoying in its own right. But what does it do? It explains why some other questionable code is alright.

You had written:

TENS_PREFIXES = ['_', '_', 'twen', 'thir', 'for',
                 'fif', 'six', 'seven', 'eigh', 'nine']

with underscores in the first two positions. There was no comment at this point. Then, down in the bowels of a function, you have a comment saying that these two bogus values are not really bogus because your code handles them separately?!?!?!?!

Why not replace '_' with 'not used' or None? That would do a far better job of making this clear. Or, just eliminate those slots entirely, and subtract 2 from the indexes. (But, frankly, I'd prefer 'not used' because that's as explicit as it's possible to be.)

And I'd prefer to see the comment read something like:

elif ones == 0:
    # After checks, 2 <= tens < 10
    return '{}ty'.format(TENS_PREFIXES[tens])

But what would be even better than that comment would be this:

elif ones == 0:
    assert(2 <= tens < 10)
    return '{}ty'.format(TENS_PREFIXES[tens])

Because that is the kind of "comment" that you know is true!

In summary, please put your comments above the affected line. It provides a warning that I might have to think hard about what's coming. And it hints where I should focus my attention. Also, don't apologize in a comment. If you find yourself explaining something, the first thing you should try to do is FIX IT. If there are funny values, try to delete them. If you can't delete them, or don't want to, then try to make them as self-explanatory as possible, so a comment isn't needed.

Some Comments on Programming Style

Knowing your limits

You handle the special case of zero. But you don't handle any other limits. For example, what if number is less than zero? Or, what if number is greater than or equal to 1 nonillion?

I'd suggest that you include nonillions in your vocabulary, just for completeness (every other array goes up to 9x). Beyond that, you should have a plan: will you raise an exception, or just print "123,456 nonillion", or return a special string: "the rent is too damn high!"? You need to handle it - pick a mechanism.

Code shape

Here's a picture of your code shape. I deleted blank lines and comments, and removed the constant lists. I then replaced every character with '#':

In general I like the fact that most of your code is close to the left edge of the screen. There are a couple of lines that are long, but only one really pushes the right edge.

With that said, you seem a little too focused on breaking code down - it's a small thing, but I think I'd prefer a few more 3-tab lines, and a few less subroutines. There are some functions that probably shouldn't be functions. In particular, your separation of 3-digits and 2-digits is a mistake, IMO. Handling the English "hundreds" is really just a matter of computing a string, since it will either be "N hundred " or it will be ''. I believe you should handle the entire three-digit "chunk" (your term) as a single function.

Use of Python

Iterators, do what they say;
Generators, let you have your way;
Dundermethods, hit you like a truck;
Curly braces -- wait! what the ...
Pythonic! Pythonic! A comprehension, not a loop body!

With due apologies to Madonna, there are some places where your code could be more Pythonic. Let's start with this:

def number_to_words(number):

    if number == 0:
        return 'zero'

    chunks = to_base_1000(number)

    chunk_words = []

    for position, chunk in enumerate(chunks):
        chunk_words.append(chunk_to_words(chunk, position))

    return ' '.join(chunk_words[::-1])

You handle the zero case just fine, but the next thing you do?

    chunks = to_base_1000(number)

You return a list of chunks. But wait! Actually, you return a list of chunks ... backwards!

You reverse the order of processing for no good reason. It's somewhat more convenient to you, in the sense that you can keep dividing by 1000 in the to_base_1000 function. But it's not actually helpful for any other part of your code. All it does is require that somewhere, another piece of code has to reverse the order of whatever data it has.

Here's a link to Ned Batchelder's Loop Like a Native presentation. Please watch it, even if you've seen it before, because it totally applies in this case!

Instead of creating a list of 3-digit chunks and returning it (backwards!), why not create a generator?

Furthermore, since a forward generator will have to know about powers of 1000, why not create a generator that will also deal with your powers-of-1000 labelling? You can kill two birds with one stone:

def number_to_chunks(num):
    """
    Convert a number to a series of 'chunks', each < 1000, with
    an accompanying text word describing the power of 1000 each 
    represents. For example, 123456789 would yield (123, 'million'),
    (456, 'thousand'), (789, '').

    """
    pass

A compliment

I'll take this opportunity to compliment this code:

def to_hundreds_tens_ones(number):
    assert(0 <= number < 1000)
    return number // 100, number // 10 % 10, number % 10

I like the assertion, and breaking the number into a tuple of digits. I'll suggest, however, that you consider calling it to_digits instead -- because how can you stand to type the current name?

Source code locality

Next, I'll point out that you have a pile of "global" variables defined that in general you only use in one place. I suggest that you redefine most of your global lists to be tuples instead, since they aren't going to grow (or shrink, or get changed) during execution. And furthermore, move the tuples into the functions that actually use them. Either treat them as constants, or as literals if they're only used once ever:

# As a constant:
ones = ('', 'one', 'two', ...)
return ones[units]

# Or as a literal:
return ('', 'one', 'two', ...)[units]

Connecting to other parts of Python

Finally, I'll point out that your code doesn't "connect" to other parts of Python. Python's string formatting provides extension points for this purpose. You could create your own string.Formatter subclass to add a {!words} conversion flag. Likewise for logging.

There is no "locale" support in your code. For example, if the current language is French, what do you do? Also, there's a difference in UK vs. American English with respect to "milliards" and Large Number handling - can you switch? Should you abort, or fail, or ...?

How about (im)precision? If I give you 1000000000000020, can I pass a flag that allows you to skip the 20 and just truncate it to the nearest billion? Or perhaps the nearest 10%?

How about access to internal data? If I pass a number, can you give me just the largest power-of-1000 word? So I can get "trillion" and print "1.2 trillion" myself?