Converting an integer to Greek and Roman numerals

Question

Yesterday I posted a question about converting an integer to it's Roman Numeral equivalent. Apparently you guys liked it and gave me some good advice, so I decided to take all your advice, and go a step further, it will now not only convert to Roman but will now also convert to Greek numerals.

I would like some critique on this project that I'm continuing.

# <~~ coding=utf-8 ~~>
import argparse


opts = argparse.ArgumentParser()
opts.add_argument("-g", "--greek", type=int,
                  help="Convert to Medieval Numerals")
opts.add_argument("-r", "--roman", type=int,
                  help="Convert to Roman Numerals")
args = opts.parse_args()


ROMAN_NUMERAL_TABLE = (
    ("~M", 1000000), ("~D", 500000), ("~C", 100000),
    ("~L", 50000), ("~X", 10000), ("~V", 5000),  # "~" indicates a Macron
    ("M", 1000), ("CM", 900), ("D", 500),
    ("CD", 400), ("C", 100),  ("XC", 90),
    ("L", 50),   ("XL", 40),  ("X", 10),
    ("IX", 9),   ("V", 5),    ("IV", 4),
    ("I", 1)
)


GREEK_NUMERAL_TABLE = (
    ("α", 1), ("β", 2), ("γ", 3),
    ("δ", 4), ("ε", 5), ("Ϝ", 6),
    ("ζ", 7), ("η", 8), ("θ", 9),
    ("ι", 10), ("κ", 20), ("λ", 30),
    ("μ", 40), ("ν", 50), ("ξ", 60),
    ("ο", 70), ("π", 80), ("ϙ", 90),
    ("ρ", 100), ("σ", 200), ("τ", 300),
    ("υ", 400), ("φ", 500), ("χ", 600),
    ("ψ", 700), ("ω", 800), ("ϡ", 900),
    ("α", 1000), ("β", 2000), ("γ", 3000),
    ("δ", 4000), ("ε", 5000), ("ϛ", 6000),
    ("ζ", 7000), ("η", 8000), ("θ", 9000)  # The Greeks weren't very creative
)


def convert_init(number, convert_to=None):
    """ Convert a number to a numeral, Greek or Roman
    >>> print(convert_init(45, convert_to=GREEK_NUMERAL_TABLE))
    ϜϜϜϜϜϜϜγ
    >>> print(convert_init(45, convert_to=ROMAN_NUMERAL_TABLE))
    XLV """
    display_numerals = []

    for numeral, value in sorted(convert_to)[::-1]:  # sort the list from largest to least
        count = number // value
        number -= count * value
        display_numerals.append(numeral * count)

    return ''.join(display_numerals)


if __name__ == '__main__':
    if args.greek:
        data = convert_init(int(args.greek), convert_to=GREEK_NUMERAL_TABLE)
        with open("greek_numerals.txt", "a+") as file_data:
            file_data.write(data)  # Write it to a file

    elif args.roman:
        data = convert_init(int(args.roman), convert_to=ROMAN_NUMERAL_TABLE)
        with open("roman_numerals.txt", "a+") as file_data:
            file_data.write(data)

    else:
        raise NotImplementedError("{} is not implemented yet".format(args))

Key points I would like to focus on, of course critique the whole program.

1. I used writing to a file because when if I output it to the console I get an IO error, basically an encoding error. Does this have to do with my computer or with the program?
2. Would there be a way for me to condense the two tuples down into one and still convert correctly?

Answer 1

Your conversion routines make no sense. I don't see why 45 should be ϜϜϜϜϜϜϜγ, as stated in your doctest. I also get this clearly wrong result:

>>> convert_init(1939, ROMAN_NUMERAL_TABLE)
'XLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXVIV'

Since you are using Unicode for the Greek numerals, I should be pedantic and point out that the Roman numerals should also use the appropriate Unicode characters.

The docstring would be better written as

def convert_init(number, convert_to=None):
    """
    Convert a number to a Greek or Roman numeral

    >>> convert_init(45, convert_to=ROMAN_NUMERAL_TABLE)
    'XLV'
    """
    …

… since the print statement obscures the fact that you are returning a string. (Based on just your docstring, it could also be returning some other kind of object, whose __str__ produces the desired output.)

What is convert_init? Did you mean convert_int?

Your args = opts.parse_args() is not guarded by if __name__ == '__main__' as it should be. You shouldn't need to cast int(args.greek) or int(args.roman), since argparse should take care of the type=int for you.

Answer 2

• I wouldn't use if __name__ == '__main__': for extended logic. I'd put just a main() in there and do the checks/call the functions from there.

• Your script is supposed to have user input and output. Having an exception is a nice way to communicate between modules/functions, but with the user you may prefer a print()

• The behavior is not entirely consistent because launching the program with an unknown option triggers a message from argparse saying unrecognized arguments (which is correct). But if you launch it with no arguments you get the exception.

• You're checking using if args.greek: and elif args.roman:, but if you give a false value, they will fail. That means that passing -r 0 on the command line will trigger the exception of not implemented. You should check against None, something like if args.greek != None:

• convert_init has a parameter convert_to with a default of None. Default values are supposed to be either valid or properly handled, and in this case None is ... well, none of the two.

• convert_to is not a proper name, because it suggests that there are two possible values, roman and greek, while in reality that's the lookup table you're going to use. I'd either rename it to symbols_lookup or (better IMO), keep the name and use it to determine which lookup table to use inside of the function.

• In convert_init you sort the values of convert_to, but they're static values, so you can sort them beforehand and use the values directly.

• You have a comment saying # "~" indicates a Macron which is useful. The other comments can (and should) go. You know that file_data.write(data) means # Write it to a file, no need to comment on that.