I have decided to give python a try and have to start out at the very basics, so I chose this well-represented kata. The code aims to convert Arabic numbers between 1 and 3999 to Roman numerals and back. Here, Roman numerals are 'well-formed', that is, no 'XM' or similar, even though I know they were occasionally used.

I took some of the test values from the publicly available Dive into Python.

I'm very interested in everything you have to say, as from what I've heard Python has quite some guidelines and best practices! Also I'm not sure whether the data structures I use are appropriate/a good choice. But here goes the code:


import re

class NumberOutOfRangeError(Exception):

class InvalidRomanNumeralError(Exception):

roman_numeral_validator = re.compile(

base_conversions = {
    1: 'I',
    4: 'IV',
    5: 'V',
    9: 'IX',
    10: 'X',
    40: 'XL',
    50: 'L',
    90: 'XC',
    100: 'C',
    400: 'CD',
    500: 'D',
    900: 'CM',
    1000: 'M'

base_numerals = {v: k for k, v in base_conversions.items()}

base_numbers = sorted(base_conversions.keys(), reverse=True)

def to_roman(arabic_number):
    if arabic_number < 0:
        raise NumberOutOfRangeError('negative values are not allowed')
    elif arabic_number == 0:
        raise NumberOutOfRangeError('zero is not allowed')
    elif arabic_number > 3999:
        raise NumberOutOfRangeError('number is too big')

    resulting_numeral = ''
    for num in base_numbers:
        while arabic_number >= num:
            arabic_number -= num
            resulting_numeral += base_conversions[num]

    return resulting_numeral

def from_roman(roman_numeral):
    if not roman_numeral_validator.match(roman_numeral):
        raise InvalidRomanNumeralError('invalid roman numeral')

    resulting_number = 0
    i = 0
    while i < len(roman_numeral):
        current_letter = roman_numeral[i]

        if i + 1 < len(roman_numeral):
            potential_base_numeral = current_letter + roman_numeral[i + 1]
            if potential_base_numeral in base_numerals:
                resulting_number += base_numerals[potential_base_numeral]
                i += 2

        resulting_number += base_numerals[roman_numeral[i]]
        i += 1

    return resulting_number


import pytest
import roman

single_digit_values = (
    (1, 'I'),
    (2, 'II'),
    (3, 'III'),
    (4, 'IV'),
    (5, 'V'),
    (6, 'VI'),
    (7, 'VII'),
    (8, 'VIII'),
    (9, 'IX')

extended_known_values = (
    (1, 'I'),
    (2, 'II'),
    (3, 'III'),
    (4, 'IV'),
    (5, 'V'),
    (6, 'VI'),
    (7, 'VII'),
    (8, 'VIII'),
    (9, 'IX'),
    (10, 'X'),
    (50, 'L'),
    (100, 'C'),
    (500, 'D'),
    (1000, 'M'),
    (31, 'XXXI'),
    (148, 'CXLVIII'),
    (294, 'CCXCIV'),
    (312, 'CCCXII'),
    (421, 'CDXXI'),
    (528, 'DXXVIII'),
    (621, 'DCXXI'),
    (782, 'DCCLXXXII'),
    (870, 'DCCCLXX'),
    (941, 'CMXLI'),
    (1043, 'MXLIII'),
    (1110, 'MCX'),
    (1226, 'MCCXXVI'),
    (1301, 'MCCCI'),
    (1485, 'MCDLXXXV'),
    (1509, 'MDIX'),
    (1607, 'MDCVII'),
    (1754, 'MDCCLIV'),
    (1832, 'MDCCCXXXII'),
    (1993, 'MCMXCIII'),
    (2074, 'MMLXXIV'),
    (2152, 'MMCLII'),
    (2212, 'MMCCXII'),
    (2343, 'MMCCCXLIII'),
    (2499, 'MMCDXCIX'),
    (2574, 'MMDLXXIV'),
    (2646, 'MMDCXLVI'),
    (2723, 'MMDCCXXIII'),
    (2892, 'MMDCCCXCII'),
    (2975, 'MMCMLXXV'),
    (3051, 'MMMLI'),
    (3185, 'MMMCLXXXV'),
    (3250, 'MMMCCL'),
    (3313, 'MMMCCCXIII'),
    (3408, 'MMMCDVIII'),
    (3501, 'MMMDI'),
    (3610, 'MMMDCX'),
    (3743, 'MMMDCCXLIII'),
    (3844, 'MMMDCCCXLIV'),
    (3888, 'MMMDCCCLXXXVIII'),
    (3940, 'MMMCMXL'),
    (3999, 'MMMCMXCIX')

def test_single_arabic_digits_are_converted_correctly():
    for digit, numeral in single_digit_values:
        assert roman.to_roman(digit) == numeral

def test_negative_number_raises_exception():
    with pytest.raises(roman.NumberOutOfRangeError):

def test_zero_raises_exception():
    with pytest.raises(roman.NumberOutOfRangeError):

def test_number_greater_than_3999_raises_exception():
    with pytest.raises(roman.NumberOutOfRangeError):

def test_numbers_are_converted_correctly():
    for digit, numeral in extended_known_values:
        assert roman.to_roman(digit) == numeral

def test_too_many_repeated_numeral_letters():
    for numeral in ('MMMM', 'DD', 'CCCC', 'LL', 'XXXX', 'VV', 'IIII'):
        with pytest.raises(roman.InvalidRomanNumeralError):

def test_numerals_are_converted_correctly():
    for digit, numeral in extended_known_values:
        assert roman.from_roman(numeral) == digit

def test_sanity_check_to_then_from_roman():
    for i in range(1, 3999):
        assert i == roman.from_roman(roman.to_roman(i))

1 Answer 1


Instead of building a dict, sorting the keys and using that sorted list of keys to get the right elements from the dict, why not use a collections.OrderedDict:

from collections import OrderedDict
base_conversions = OrderedDict(
    (1000, 'M'),
    (5, 'V'),
    (4, 'IV'),
    (1, 'I'))

Same for base_numerals.

In python it is considered more pythonic to iterate over the elements of a list. Consider the following implementation where I used zip to have both the current and the following numeral to compare:

def roman_to_int(roman_numeral):
    """Convert from Roman numerals to an integer. Number must be < 4000."""
    if not roman_numeral_validator.match(roman_numeral):
        raise InvalidRomanNumeralError('invalid roman numeral')

    numbers = [base_numerals[char] for char in roman_numeral]
    total = 0
    for current_n, next_n in zip(numbers, numbers[1:]):
        if current_n >= next_n:
            total += current_n
            total -= current_n
    return total + numbers[-1]

In addition, you should add a docstring to every function you write. This allows you to do e.g. help(roman_to_int), which will print Convert from Roman numerals to an integer. Number must be < 4000.

  • \$\begingroup\$ Thanks for the answer, I'm trying to understand your implementation of the algorithm right now. It only seems to work for Roman numerals with more than one letter for me though, because otherwise next_n has not been assigned. It also taught me that I have to read up on scope in Python ;) \$\endgroup\$ Commented Aug 16, 2016 at 19:38
  • \$\begingroup\$ Yes, well spotted with the single numeral...edited a fix for that. Indeed, in most programming languages next_n would not be defined at the end, but here it was. \$\endgroup\$
    – Graipher
    Commented Aug 17, 2016 at 11:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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