Logic Error in parsing file with python: KeyError: 'O'

Question

so I'm trying to open a text file with a poem and see how many times I can spell the word "GOOD" with the letter in the text file in each line, but I get the following error:

Traceback (most recent call last):
  File "./soup.py", line 11, in <module>
    print( "\n".join( [("Case #%d: %d" % (i, parse(file[i]))) for i in range(1, len(file))]))
  File "./soup.py", line 7, in parse
    d['O'] /= 2
KeyError: 'O'

source:

#!/usr/bin/python

def parse(string):
    d = {'G' : 0, 'O' : 0, 'D' : 0}
    d = {s: string.count(s) for s in string if s in d } 
    d['O'] /= 2
    return min(d.values())

file = open("poem.txt").read().split('\n')
print( "\n".join( [("Case #%d: %d" % (i, parse(file[i]))) for i in range(1, len(file))]))

Answer 1

FWIW, I would write this using the Counter object:

from collections import Counter

def spellcount(string, wanted):
    wanted_counts = Counter(wanted)
    have_counts = Counter(string)
    return min(have_counts[c]//wanted_counts[c] for c in wanted_counts)

wanted = "GOOD"
with open("poem.txt") as fp:
    for i, line in enumerate(fp):
        print("Case", i, ":", spellcount(line, wanted))

Counter behaves as a defaultdict, e.g.

>>> from collections import Counter
>>> Counter('GOOD')
Counter({'O': 2, 'G': 1, 'D': 1})
>>> Counter('GOOD')['i']
0

Answer 2

Are you sure your line 7 reads d['O']?

The error message suggests it reads d['C']

The problem is that if the line contains no 'O' characters then 'O' will not be in d and this gives an error.

The second time d is defined will create a new dictionary which will not include the 'O' key.

def parse(string):
    d = {'G' : 0, 'O' : 0, 'D' : 0}
    d = {s: string.count(s) for s in string if s in d }
    try:
        d['O'] /= 2
    except KeyError:
        return 0
    return min(d.values())

file = open("test1.py").read().split('\n')
print( "\n".join( [("Case #%d: %d" % (i, parse(file[i]))) for i in range(1, len(file))]))

(You might find it more efficient to do d = {s: string.count(s) for s in d })

(I also love the alternative suggestion of using collections.Counter. However, if you are interested in speed then my timing measurements show that for a 10 million character string it takes 3 seconds to make the Counter object, but only 0.012 seconds per call to string.count)

Answer 3

Use 'GOD' instead of string in your dictionary comprehension, because not every line will include the 'O'.

def parse(string):
    d = {s: string.count(s) for s in 'GOD'}
    d['O'] /= 2
    return min(d.values())

Answer 4

The problem is if you encounter a line that does not countain an 'O', that key won't exist. You can solve this using dict.get. dict.get does not raise an exception if a key doesn't exist but returns its second argument, in this case 0:

def parse(string):
    d = {s: string.count(s) for s in string if s in 'GOD'} 
    d['O'] = d.get('O', 0) / 2
    return min(d.values())

Note the above only matches uppercase letters, if you want both upper- and lowercase you can do this:

def parse(string):
    string = string.upper()
    d = {s: string.count(s) for s in string if s in 'GOD'} 
    d['O'] = d.get('O', 0) / 2
    return min(d.values())

Edit

Of course, using d = {s: string.count(s) for s in 'GOD'} avoids the issue entirely and is more concise too. I suggest using @Gandaro's answer.