Using 'get' method of a dictionary to select case. Why does it keep selecting the wrong thing?

Question

I'm using a dictionary's get() method to choose a case based on a variable key value. Problem is, it keeps choosing the wrong case for the key. I'm not sure if I'm using the right words to explain it, but the code should make it clear enough.

di_eqns = {"PSME": Pred_PSME(liLn), "ABAM":Pred_ABAM(liLn), \
           "ABCO":Pred_Abies(liLn), "ABGR":Pred_Abies(liLn), \
           "ABLA":Pred_Abies(liLn), "ABMA":Pred_Abies(liLn), \
           "ABSH":Pred_Abies(liLn), "ABPR":Pred_ABPR(liLn), \
           "CADE27":Pred_Cedar(liLn), "CHLA":Pred_Cedar(liLn), \
           "CHNO":Pred_Cedar(liLn), "THPL":Pred_Cedar(liLn), \
           "LALY":Pred_LALY(liLn), "LAOC":Pred_LAOC(liLn), \
           "PIEN":Pred_PIEN(liLn), "PISI":Pred_PISI(liLn), \
           "PIAL":Pred_Pinus(liLn), "PIJE":Pred_Pinus(liLn), \
           "PIMO3":Pred_Pinus(liLn), "PICO":Pred_PICO(liLn), \
           "PILA":Pred_PILA(liLn), "PIPO":Pred_PIPO(liLn), \
           "TSHE":Pred_TSHE(liLn), "TSME":Pred_TSME(liLn), \
           "JUOC":Pred_JUOC(liLn), "TABR":Pred_TSHE(liLn), \
           "ALRU2":Pred_ALRU2(liLn), "ACCI":Pred_SMBGr(liLn), \
           "ACGL":Pred_SMBGr(liLn), "ACMA3":Pred_ACMA3(liLn), \
           "CONU4":Pred_ACMA3(liLn), "ALRH2":Pred_AACWGr(liLn), \
           "SASC":Pred_AACWGr(liLn), "SALIX":Pred_AACWGr(liLn), \
           "ARME":Pred_ARME(liLn), "BEPA":Pred_BEPA(liLn), \
           "BEPAC":Pred_BEPA(liLn), "CHCH7":Pred_CHCH7(liLn), \
           "FRLA":Pred_MHGr(liLn), "PREM":Pred_MHGr(liLn), \
           "PRVI":Pred_MHGr(liLn), "PRUNU":Pred_MHGr(liLn), \
           "LIDE3":Pred_LIDE3(liLn), "POBAT":Pred_POBAT(liLn), \
           "POTR5":Pred_POTR5(liLn), "QUCH2":Pred_QUCH2(liLn), \
           "QUGA4":Pred_QUGA4(liLn), "QUKE":Pred_QUKE(liLn), \
           "UMCA":Pred_UMCA(liLn)}
try: li_pred = di_eqns.get(liLn[2])

Here is the list (a line from a text file) that the key was taken from when it went to the wrong function (it went to Pred_PSME):

['1347189', '20571', 'PIEN', 'Picea engelmannii', 'Y', 'Y', '66.294',
 '0.3452', '35.7418', '', '5.0996', '0', '1', '1.1324', '3.2803', '2.3057',
 '16.7282', '5520.306', '127.30', '70.73', '92.10', '0.00', '5.68', '2.37',
 '7.25', '79.16', '76.79', '290.15', '219.41', '135.37', '0.00', '0.00',
 '951.78', '719.74', '259.68', '251.89', '444.05', '0.00', '0.00', '',
 '12.70', '10.16', '15.24', '0.02', '0.04', '0.19', '0.19', '0.00', '0.00',
 '0.42', '0.32', '0.11', '0.11\r\n']

Anyone know why this is happening and how to fix it? Let me know if there is any additional info you need. Also, I'm open to suggestions for a better name for this question.

ADDENDUM: Sorry I didn't make this question clearer originally. I'm still a beginner in python and new to Stack Overflow; I wasn't aware it was ambiguous and I was under the impression we were supposed to make our questions as concise as possible and irrelevant details were undesirable.

This is the goal of the program, to read lines from a file, each line being the measurements from a tree from one of multiple forest inventory plots, and for each line, run certain species specific equations on the tree represented by the first few values in the line, then add up the results of the equations for all the trees in each plot.

The section of code I included above reads the third value from the line, which is the species code for the tree represented in the line, and passes the line to the function that contains the specific equations for that species of tree.

Based on the answers I've gotten already, it sounds like when the dictionary definition above is read, it runs through every single function listed. I wanted something that would basically do what select case would do in other languages. I got the idea of using a dictionary and the get method from another Q&A on Stack Overflow.

I only want to run the function that matches the species code in the list of the line each time the program loops over a new line. What I've noticed happening is that it seems that this dictionary is passing lines to the wrong function because I put an error catching bit of code in the Pred_PSME function that prints the line value when there is an error (there was also an error that kept popping up in calculating one of the equation values in Pred_PSME, which is why I added that debugging code), and it printed the line included above.

Maybe I'm misunderstanding how the dictionary full of function names works. I guess my question would be, write code that would send the list of the elements in a line to the function that matches the species code given in liLn[2]?

In another language you would use select case on the value of liLn[2], with each case being one of the keys listed in the dictionary and the commands for each case being the commands in the functions whose names are listed in the values of the dictionary.

Here is the code that leads up to the dictionary and more context for the dictionary:

def Main():
    srcf = open(bkp, 'r')
    old_fcid = 0
    li_fBQI = []
    i = 0
    for line in srcf:
        liLn = line.split(',')
        liLn = stripquotes(liLn)
        #Check if it's the first line, if so, jump to next line.
        if len(liLn) < 2: continue
        if HdrLine(liLn[:2]): continue
        fcid = liLn[1]
        #Check line fcid against last fcid
        if old_fcid != 0 and fcid != old_fcid:
            #Write the FCID BQI tallies to file
            Write_fBQIs(old_fcid, li_fBQI)
            #Reset FCID BQI tallies
            li_fBQI = []
        old_fcid = fcid
        #Calc BQI's for the tree
        li_tBQI = BQI_Calc(liLn)
        #Add tree BQI's to the FCID tallies
        li_fBQI = AddBQI(li_tBQI, li_fBQI)
        if i % 1000 == 0: print 'Finished line #' + str(i)
        i += 1
    #ADD: Write last FCID's BQI's to file
    Write_fBQIs(old_fcid, li_fBQI)
    srcf.close()

def BQI_Calc(liLn):
    di_eqns = {"PSME": Pred_PSME(liLn), "ABAM":Pred_ABAM(liLn), \
               "ABCO":Pred_Abies(liLn), "ABGR":Pred_Abies(liLn), \
               "ABLA":Pred_Abies(liLn), "ABMA":Pred_Abies(liLn), \
               "ABSH":Pred_Abies(liLn), "ABPR":Pred_ABPR(liLn), \
               "CADE27":Pred_Cedar(liLn), "CHLA":Pred_Cedar(liLn), \
               "CHNO":Pred_Cedar(liLn), "THPL":Pred_Cedar(liLn), \
               "LALY":Pred_LALY(liLn), "LAOC":Pred_LAOC(liLn), \
               "PIEN":Pred_PIEN(liLn), "PISI":Pred_PISI(liLn), \
               "PIAL":Pred_Pinus(liLn), "PIJE":Pred_Pinus(liLn), \
               "PIMO3":Pred_Pinus(liLn), "PICO":Pred_PICO(liLn), \
               "PILA":Pred_PILA(liLn), "PIPO":Pred_PIPO(liLn), \
               "TSHE":Pred_TSHE(liLn), "TSME":Pred_TSME(liLn), \
               "JUOC":Pred_JUOC(liLn), "TABR":Pred_TSHE(liLn), \
               "ALRU2":Pred_ALRU2(liLn), "ACCI":Pred_SMBGr(liLn), \
               "ACGL":Pred_SMBGr(liLn), "ACMA3":Pred_ACMA3(liLn), \
               "CONU4":Pred_ACMA3(liLn), "ALRH2":Pred_AACWGr(liLn), \
               "SASC":Pred_AACWGr(liLn), "SALIX":Pred_AACWGr(liLn), \
               "ARME":Pred_ARME(liLn), "BEPA":Pred_BEPA(liLn), \
               "BEPAC":Pred_BEPA(liLn), "CHCH7":Pred_CHCH7(liLn), \
               "FRLA":Pred_MHGr(liLn), "PREM":Pred_MHGr(liLn), \
               "PRVI":Pred_MHGr(liLn), "PRUNU":Pred_MHGr(liLn), \
               "LIDE3":Pred_LIDE3(liLn), "POBAT":Pred_POBAT(liLn), \
               "POTR5":Pred_POTR5(liLn), "QUCH2":Pred_QUCH2(liLn), \
               "QUGA4":Pred_QUGA4(liLn), "QUKE":Pred_QUKE(liLn), \
               "UMCA":Pred_UMCA(liLn)}
    try:
        li_pred = di_eqns.get(liLn[2])
    except:
        print '\n\n', 50*'-', 'ERROR: Couldn\'t find equations for the spp',\
              'code:', liLn[2]

    #Calc derivative quantities

    .....and the function continues from there

Here is an example of the functions listed in the dictionary. Pred_PSME is the one that I mentioned above. An error kept occuring in the stem bark (sb) calculation, so I added some debugging code, thats how I found out that it was sent the PIEN tree, which is why I started this question:

def Pred_PSME(liLn):
    dbh = float(liLn[6])
    fol = exp(log(float(liLn[19])) + pqty(.0627, 123))
    lbr = exp((-3.6941 + 2.1382*log(dbh)) + pqty(.057, 123))
    dbr = exp((-3.529 + 1.7503*log(dbh)) + pqty(.079, 85))
    sw = exp(log(float(liLn[20])) + pqty(.0311, 99))
    try:
        sb = exp(log(float(liLn[21])) + pqty(.0324, 99))
    except:
        Badtree(liLn)
        print dbh, liLn
    ts = exp(-3.0396 + 2.5951*log(0.865 * mtd) + pqty(.0311, 99)) +\
         exp(-4.3103 + 2.4300*log(0.865 * mtd) + pqty(.0324, 99))
    br = lbr +dbr
    agb_nf = br + sw + sb
    agb = agb_nf + fol
    return [fol, br, sw, sb, agb, agb_nf, ts]

def Pred_ARME(liLn):
    dbh = float(liLn[6])
    agb_nf = float(liLn[27]) + tvariate(57) * 1.23 * (1 + 1/60)**(1/2)
    agb = agb_nf / (1 - folpct)
    fol = agb - agb_nf
    br = sw = sb = ts = 0
    return [fol, br, sw, sb, agb, agb_nf, ts]

I hope this made things clearer. Please let me know if there are any more questions. BTW, what does everyone mean when they say "code smell"? Also, I'm having some trouble with formatting text for this site. Is there a way to ident a code block on this site 4 spaces without manually adding 4 spaces to every line? Thanks.

I took the dictionary that you have here (replacing all of the dictionary values with strings) and ran `print di_eqns.get('PIEN')`. I received as a response `'Pred_PIEN(liLn)'`. What were you expecting as a result? — Mark Hildreth, Mar 20 '11 at 07:52
I think it odd that your dictionary contains the *results* of functions rather than the functions themselves. — Steven Rumbalski, Mar 20 '11 at 08:11
This question is not coherent. Please provide the data of the real dict not not a definition like "QUGA4":Pred_QUGA4(liLn). We can not smell what Pred_UMCA(...) will return. Otherwise this question deserves a -1. — , Mar 20 '11 at 08:12
Are you sure that the dict above is actually the one that ran? Is there any other code between `di_eqns = {......}` and `try:`? How do you know that it went to the wrong function? — John Machin, Mar 20 '11 at 08:38
site note: don't end lines with '\', there is no need to here. — tokland, Mar 20 '11 at 09:23
@cfwschmidt What does it mean : _"to go to a wrong function"_ ? — eyquem, Mar 20 '11 at 14:41
pynator, that is the real dictionary. I'm new here, and like I said in my question, I'm not sure how to better explain the question without giving the entire 10 pages of code from the program. If you don't like the question and want to criticize a new member go hassle someone else. — cfwschmidt, Mar 21 '11 at 03:03
All of the values in the dictionary are names of functions to be called if the key is given, not the results of functions, Steve. It's like a select case statement in other languages. Mark, when 'PIEN' is passed to the get method, I expect it to run "Pred_PIEN", but instead it ran "Pred_PSME". John, I know this because there was an unrelated error in the Pred_PSME function and the 'except' code for that function prints out the list that the function encounters an error processing. This is the only dictionary I use in my program, so I'm pretty sure it's the only one that ran. — cfwschmidt, Mar 21 '11 at 03:08
@cfwschmidt: `Pred_PSME(liLn)` is the RESULT of a function call. `Pred_PSME` is the NAME of a function. You are gravely mistaken. **There is a very strong presumption that it would have run ALL of your functions, but it was stopped by an exception in the Pred_PSME function**. You don't need to show all 10 pages of your code; just answer this question: Is there any code between `di_eqns = {.....}` and the `try:` statement? Also, have you actually read @martineau's answer? My answer? — John Machin, Mar 21 '11 at 05:57
@cfwschmidt: BTW, you can indent blocks of code pasted into your answer by selecting all the lines containing it and then clicking on the little `{ }` icon above the answer text box. — martineau, Mar 21 '11 at 07:12
@cfwschmidt: Also, based on seeing the additional code, you don't need to create the `di_eqns` dictionary every time you call the `BQI_Calc()` function since it's (now) just a dispatch table pointing to a bunch of statically defined `Pred_XXXX` functions. — martineau, Mar 21 '11 at 07:59

martineau · Accepted Answer · 2011-03-24T13:36:40.177

3

I believe the problem is that all the Pred_XXXX(liLn) values -- which appear to be function calls -- in the di_eqns dictionary are being replaced with their return values when you create it...which of course will depending on the value of liLn at the time of construction, not at the time you call get().

Something like the following would delay the calls to the Pred_XXXX() functions. Basically it first looks up the function to call based on the third item in the list from the text file, and then passes the whole list to the function:

di_eqns = {"PSME":Pred_PSME, "ABAM":Pred_ABAM,
           "ABCO":Pred_Abies, "ABGR":Pred_Abies,
           "ABLA":Pred_Abies, "ABMA":Pred_Abies,
           "ABSH":Pred_Abies, "ABPR":Pred_ABPR,
           "CADE27":Pred_Cedar, "CHLA":Pred_Cedar,
           "CHNO":Pred_Cedar, "THPL":Pred_Cedar,
           "LALY":Pred_LALY, "LAOC":Pred_LAOC,
           "PIEN":Pred_PIEN, "PISI":Pred_PISI,
           "PIAL":Pred_Pinus, "PIJE":Pred_Pinus,
           "PIMO3":Pred_Pinus, "PICO":Pred_PICO,
           "PILA":Pred_PILA, "PIPO":Pred_PIPO,
           "TSHE":Pred_TSHE, "TSME":Pred_TSME,
           "JUOC":Pred_JUOC, "TABR":Pred_TSHE,
           "ALRU2":Pred_ALRU2, "ACCI":Pred_SMBGr,
           "ACGL":Pred_SMBGr, "ACMA3":Pred_ACMA3,
           "CONU4":Pred_ACMA3, "ALRH2":Pred_AACWGr,
           "SASC":Pred_AACWGr, "SALIX":Pred_AACWGr,
           "ARME":Pred_ARME, "BEPA":Pred_BEPA,
           "BEPAC":Pred_BEPA, "CHCH7":Pred_CHCH7,
           "FRLA":Pred_MHGr, "PREM":Pred_MHGr,
           "PRVI":Pred_MHGr, "PRUNU":Pred_MHGr,
           "LIDE3":Pred_LIDE3, "POBAT":Pred_POBAT,
           "POTR5":Pred_POTR5, "QUCH2":Pred_QUCH2,
           "QUGA4":Pred_QUGA4, "QUKE":Pred_QUKE,
           "UMCA":Pred_UMCA}

try: li_pred = di_eqns.get(liLn[2])(liLn)

However, without additional information about what you're trying to accomplish, it's difficult to make any definitive suggestions...

edited Mar 24 '11 at 13:36

answered Mar 20 '11 at 08:19

martineau

119,623
25
170
301

I think we've all detected the same code-smell, but there's not enough code given to say definitively that this is what is happening. – Steven Rumbalski Mar 20 '11 at 08:30
So let me get this straight. The way I had it before, it was running every function each time that dictionary was defined? And now the dictionary just points to the functions and the function is only called with the get method and parameter together? – cfwschmidt Mar 21 '11 at 06:33
@cfwschmidt: Partially correct. Afterwards, the dictionary no longer points to the functions at all, just objects created to hold the result of calling the functions when it was created. In your original code the `get()` function was being called with the something from the current value of `liLn`, but that just gives you the result of what the variable had in it when the dictionary was created which might be different. – martineau Mar 21 '11 at 06:41
@cfwschmidt: FWIW, after reviewing the additional information you added to your question, I'm pretty sure what's in my answer is the root of your problem -- it's the equivalent of the `select case` you mentioned. – martineau Mar 21 '11 at 07:16
@cfwschmidt: Just noticed your code seems to have my suggestion in it. Are you still encountering the same problem or is something else happening now? – martineau Mar 21 '11 at 07:22
@cfwschmidt: It was doing exactly what I told you in my answer. @martineau: You seem to believe that his dict was set up only once. According to his code, it is being set up once per input line. How do you account for it to be appearing to call the wrong function? – John Machin Mar 21 '11 at 07:24
It's hard to tell what going wrong because the `get()` is wrapped in a `try/except` that traps **all** exceptions and regardless of what the problem actually was prints an error message saying "Couldn't find equations for the sppcode from the current line" where "sppcode" is the `liLN[2]` value. – martineau Mar 21 '11 at 07:32
@John Machin: It's not clear that it is (or even was) calling the wrong function -- see my previous comment -- and judging from the contents of the sample `liLn` list elements and how they are used in the various `Pred_XXXX` functions, there appear to be many ways for one of them to throw some unrelated exception that is being misinterpreted. – martineau Mar 21 '11 at 07:50
@martineau I accidentally included your code suggestion in my addendum. It has been seeming to work since I added it, but I just wanted to give it a little more testing to make sure it really was working and it wasn't just screwing up silently. I'm confident now that you answered my question. – cfwschmidt Mar 24 '11 at 04:37
@cfwschmidt: Glad to hear it's working for you. A suggestion: You probably should also get rid of that bare `except:` following the indirect call through `get()` to your `Pred_XXXX()` functions because it can hide or lead to a misinterpretation of errors. See [Idioms and Anti-Idioms in Python](http://docs.python.org/howto/doanddont.html#except) in the docs as well as the answers to the Stackoverflow question [about catching ANY exception](http://stackoverflow.com/questions/4990718/python-about-catching-any-exception) for more information. – martineau Mar 24 '11 at 13:31

John Machin · Answer 2 · 2011-03-20T09:45:50.137

If di_eqns is set up ONCE, then the values in that dict will depend on the value of liLn at setup time.

If di_eqns is being set up once per input record, that's not very efficient. Creating a dictionary of 49 entries is wasteful but tolerable one per record. But it's really worse in this case: it evaluates all 49 function calls then gets one result when it should evaluate only one. That's grossly wasteful. If any of those functions have side effects e.g. they alter global state, you have a gross logic problem on top of a gross efficiency problem.

I suggest that you set up your dict once, like this:

di_eqns = {
    # for better legibility, arrange the following lines in alphabetical key order;
    # then a duplicate key entry is easier to see.
    "PSME": Pred_PSME,
    "ABAM": Pred_ABAM,
    "ABCO": Pred_Abies,
    "ABGR": Pred_Abies,
    # et cetera
    }

and for each record do something like this:

if debug: print "liLn", liLn
handler_func = di_eqns[liLn[2]] # be prepared for KeyError here
if debug: print "handler", repr(handler_func)
li_pred = handler_func(liLn)
if debug: print "li_pred", repr(li_pred)

If the problem hasn't already gone away, the print statements should help you work out what the cause is. If you want more help, you'll need to show us the code for the PIEN function and the code for the PSME function, plus the code for your mainline.

Edit More code hygiene: Just had a look at "line from text file" ... ends with '0.32', '0.11', '0.11\r\n'] -- looks like you ought to be using the csv module to read your file, instead of

for guff in f:
    liLn = guff.split(',')

Thanks for your helpful comments and one of the correct answers. Would have given you the accepted answer, but martineau's was oldest. I've made the change you suggested to my code, so it only defines the dictionary once. As for the csv reader, I am trying to learn how to use it but have had some problems with it. Apparently I am misunderstanding the way the module works. I may have to ask another question after I do some research on it. — cfwschmidt, Mar 24 '11 at 04:36

Using 'get' method of a dictionary to select case. Why does it keep selecting the wrong thing?

2 Answers2