2

I am new to Python and have successfully created the following statement that writes a change period value (01 or 02) based on the municipality and date fields. I am wondering if anyone could point towards a more efficient way to do this, perhaps reducing to only the first if and elif statements since the criteria to be included under "01" or "02" are the same for kitchener, cambridge, and waterloo?

pre-code:

def ifBlock(munic, time): 
    if (munic == 'kitchener' and time == "2003" or munic == 'kitchener' and time == "2004" or munic == 'kitchener' and time == "2005" or munic == 'kitchener' and time == "2006"):
        return "01"
    elif (munic == 'kitchener' and time == "2007" or munic == 'kitchener' and time == "2008" or munic == 'kitchener' and time == "2009" or munic == 'kitchener' and time == "2010"):
        return "02"
    elif (munic == 'cambridge' and time == "2003" or munic == 'cambridge' and time == "2004" or munic == 'cambridge' and time == "2005" or munic == 'cambridge' and time == "2006"):
        return "01"
    elif (munic == 'cambridge' and time == "2007" or munic == 'cambridge' and time == "2008" or munic == 'cambridge' and time == "2009" or munic == 'cambridge' and time == "2010"):
        return "02"
    elif (munic == 'waterloo' and time == "2003" or munic == 'waterloo' and time == "2004" or munic == 'waterloo' and time == "2005" or munic == 'waterloo' and time == "2006"):
        return "01"
    elif (munic == 'waterloo' and time == "2007" or munic == 'waterloo' and time == "2008" or munic == 'waterloo' and time == "2009" or munic == 'waterloo' and time == "2010"):
        return "02"
    else: 
        return "other"

Code:

ifBlock(!MUNIC!, !TIME!)
  • 1
    More efficient? My guess is being explicit is quite performant. The logic isn't that involved. You could micro-optimize and put the values found more often first, but you may not see any difference. Do you mean to just make it more readable? – Richard Morgan Jan 7 at 20:21
2

More efficient? My guess is your code is pretty efficient in terms of churning through data. Explicit can be quite performant.

If you'd like a bit more readability, you could try something like:

accepted_munics = {'kitchener', 'cambridge', 'waterloo'}
one_times = {'2003', '2004', '2005', '2006'}
two_times = {'2007', '2008', '2009', '2010'}


def ifBlock(munic, time):
    if munic in accepted_munics:
        if time in one_times:
            return '01'
        if time in two_times:
            return '02'
    return 'other'

Note: this is untested.

If you don't know Python, this may actually be less readable. (What are those squiggly brackets for?) The logic is a bit more compact, so expressing why a value is set seems more straight-forward.

A couple of comments:

1) Any real performance measuring must be tested. The following is just a guideline: Dictionaries and sets in Python are fast. Faster than lists. So we have three sets defined and look in them for our values.

2) Are the time values really strings? You treat them like strings, so the code above does the same. If they are numbers, get rid of all the single quotes.

3) Your catch-all return of 'other' is a good idea, but you lose the ability to know if the year was outside the expected range or the municipality. This may not be a concern.

  • 1
    I like this best. IMO it could be a bit more readable converting time to an int since the ranges are consecutive and you could do e.g. if 2003 <= time < 2007:. Performance-wise I dunno how that'd compare to sets of strings, but as you said it probably wouldn't make a noticeable difference in any case – mikewatt Jan 8 at 1:18
  • @gberard I did that first but it slowed down by a fraction. 😁 Good suggestion for readability! – Richard Morgan Jan 8 at 1:23
2

More easy to read and very close to your approach (but I guess not faster/more efficient):

def ifBlock(munic, time): 
    if munic == 'kitchener' and time in ["2003", "2004", "2005", "2006"]:
        return "01"
    elif munic == 'kitchener' and time in ["2007", "2008", "2009", "2010"]:
        return "02"
    # ...
    elif munic == 'waterloo' and time in ["2007", "2008", "2009", "2010"]:
        return "02"
    else: 
        return "other"
1

Try to use a nested python dictionary, it will make it more readable/tidy:

def ifBlock(munic, time): 
    nested_dict = { {'munic1': 'time1'}: '01',
                {'munic2': 'time2'}: '01',
                {'munic3': 'time3'}: '01',
                {'munic4': 'time4'}: '02',
                {'munic5': 'time5'}: '02',
                {'munic6': 'time6'}: '02'
                ...
                }
    returnVal = ''

    try:
       # key exists in dict
       returnVal = nested_dict [{'munic':'time'}]
    except KeyError:
       # key doesn't exist in dict
       returnVal = 'other'
return returnVal 

Not sure if this is what you looking for..

0

This code may not be the best one to use situationally depending on your other records, but it's the easiest one for me to understand based on what I see (using lists and nested if-statements):

if munic in ['kitchener', 'cambridge', 'waterloo']:
    if time in list(range(2007, 2011)):
        return "02"
    elif time in list(range(2003, 2007)):
        return "01"
else:
    return "other"

Note that when using range, 2011 and 2007 are upper-bounds, but the last item in the list will actually be 2010 and 2006, respectively.

Your Answer

By clicking "Post Your Answer", you agree to our terms of service, privacy policy and cookie policy

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