Finding the date from "2nd Friday of X month"-style input

Question

I've been working through some puzzles from exercism.io. Current puzzle asks me to return a date() from input that essentially says "The 3rd Friday of December" or "The 1st Monday of October". I'd love to hear some feedback on how to clean this code up:

def meetup_day(year, month, weekday, spec_weekday):
    import calendar
    from datetime import date

    last_day = calendar.monthrange(year, month)[1]
    wkday = {'Monday': 0, 'Tuesday': 1, 'Wednesday': 2, 'Thursday': 3, 'Friday': 4, 'Saturday': 5, 'Sunday': 6}
    schedule_day = wkday[weekday]

    if spec_weekday == "teenth":
        check_range = range(13, 20)
    elif spec_weekday == "last":
        check_range = range(last_day - 6, last_day + 1)
    else:
        spec_weekday = int(spec_weekday[0:1])
        check_range = range(7 * spec_weekday - 6, 7 * spec_weekday + 1)

    for index in check_range:
        if index > last_day:
            break
        if schedule_day == calendar.weekday(year, month, index):
            schedule_day = index

    return date(year, month, schedule_day)

meetup_day(2014, 3, "Friday", "2nd")
meetup_day(2013, 6, "Wednesday", "4th")
meetup_day(2013, 12, "Monday", "1st")
meetup_day(2015, 5, "Tuesday", "teenth")
meetup_day(2015, 4, "Thursday", "last") 

I'm still fairly new to Python, and datetime stuff throws me through a loop. I chose to use calendar as well as date because I thought the syntax was easier to understand. However, I'm definitely open to ideas on more efficient ways to do this.

Quick note: The "teenth" thing:

Note that "Monteenth", "Tuesteenth", etc are all made up words. There was a meetup whose members realised that there are exactly 7 days that end in '-teenth'. Therefore, one is guaranteed that each day of the week (Monday, Tuesday, ...) will have exactly one date that is named with '-teenth' in every month.

Answer 1

Your code is pretty clear but can be improved using builtin capabilities of calendar: the monthcalendar method return a list of list representing each week of the selected month (zeroes are days that are not in the month). By filtering these weeks using the index of the day you are looking for, you get all the possible day of the month that are the requested day. You can then just pick the right one into this list.

Python related remarks:

• Put your import at the beginning of the file, not into functions. It will slow you down if you are heavily calling functions that contain import statements;
• wkday is a constant and must be declared as such: outside of the function and with proper case;
• there is no really need to make wkday a dictionary given that you can get the same information it holds using .index on a list;
• [optional] depending on your needs, you may want to perform proper input validations.

---

import calendar
from datetime import date

DAY_NAMES = [day for day in calendar.day_name]

def meetup_day(year, month, weekday, spec_weekday):
    day_index = DAY_NAMES.index(weekday)
    possible_dates = [
        week[day_index]
        for week in calendar.monthcalendar(year, month)
        if week[day_index]] # remove zeroes

    if spec_weekday == 'teenth':
        for day_num in possible_dates:
            if 13 <= day_num <= 19:
                return date(year, month, day_num)
    elif spec_weekday == 'last':
        day_index = -1
    elif spec_weekday == 'first':
        day_index = 0
    else:
        day_index = int(spec_weekday[0]) - 1
    return date(year, month, possible_dates[day_index])

meetup_day(2014, 3, "Friday", "2nd")
meetup_day(2013, 6, "Wednesday", "4th")
meetup_day(2013, 12, "Monday", "1st")
meetup_day(2015, 5, "Tuesday", "teenth")
meetup_day(2015, 4, "Thursday", "last")

Answer 2

Well my first question is, what's a spec_weekday? It can apparently be "2nd", "4th" as well as "teenth" or "last". What can't it be? Well it can't be any string besides "teenth" or "last" that doesn't begin with a number. Not even whitespace apparently. None of this is explicitly told to the user, you don't even raise helpful errors. If I tried to do this:

meetup_day(2013, 6, "Sunday", "last Sunday")

I get this unhelpful error:

Traceback (most recent call last):
  File "<pyshell#2>", line 1, in <module>
    meetup_day(2013, 6, "Sunday", "last Sunday")
  File "<pyshell#1>", line 14, in meetup_day
    spec_weekday = int(spec_weekday[0:1])
ValueError: invalid literal for int() with base 10: 'l'

I can figure out that 'l' is the first letter of my spec_weekday, but that requires investigation. Instead, you should have a docstring that explains what can be passed to parameters. Docstrings are programmatically accessible comments that tell users how to interact with your code. It's a good idea to have one in most functions, especially if the execution and arguments are non-obvious.

def meetup_day(year, month, weekday, spec_weekday):
    """Returns date object from a wordy description of a date

    Eg. "Last Sunday in November 2013" could be entered with:
    meetup_day(2013, 11, "Sunday", "last")

    spec_weekday can be an ordinal number, like 1st, 2nd, 3rd etc.
    or it can be "last" or "teenth"
    """

Now, you'd probably want to explain "teenth". As it is, I don't entirely follow it. I was trying to grasp it from just the code but it really didn't make sense, even though I did understand what it did. The requirement to include it is strange enough that you ought to clearly document it.

And when invalid input is entered you should actually respond with a clear message about it, so the user quicker realises their mistake. Something like this:

    try:
        spec_weekday = int(spec_weekday[0:1])
    except ValueError:
        raise ValueError("spec_weekday must be 'teenth', 'last' or begin with a number,")

Another problem with your input treatment, ' 4th' would be treated as invalid because of the whitespace. If you just call strip() on a string, it will remove all whitespace from the start and end. So even if the user bizarrely threw in spaces, tabs and newlines then you can get the valid input out of it. Likewise for testing against strings, you should call .lower() which will reduce all the characters to lowercase.

spec_weekday = spec_weekday.strip().lower()

With that line at the start, even " \n \t TEenTh " could be evaluated correctly.

Now, I actually think using a dictionary for your wkday is a good idea, but you have to do more to implement it correctly. One strength that a dictionary has is the ability to have multiple keys point to the same value. In your case, you can use it for multiple ways to pass the day value. For example:

wkday = {'monday': 0, 'mon': 0,

Now it's even easier to pass valid input, and since Python's a forgiving language it's worth following the paradigm. Especially as you could accept other languages or methods of representing days. Note I also made both keys lowercase, I think you should also strip().lower() the keys you pass to the dictionary. And again, you could try except KeyError to raise a proper ValueError about how that's not a recognised day.

It's bizarre that you have a dictionary for days but months must be integers. Why not have another dictionary for months? Likewise, you could make spec_weekday's special values a dictionary. And if the key doesn't exist in that dictionary then you can try calling int instead. Also, int(spec_weekday[0:1]) is the same as int(spec_weekday[0]) (and also int(spec_weekday[:1])).

And lastly, weekday is a confusing term as it means Monday-Friday. Unfortunately the slightly long day_of_week is the most clear way to name this, or day_name at least, though that's less clear to read.