3
\$\begingroup\$

I'm trying to do some datetime processing. The input is a string of the form 20141005 ('%Y%m%d') and I want to return a data structure that stores the day before the input date, the day from the input, and the day after along with the day after that in a %y%m%d format (141005)

This is my function right now, and I'm wondering if there's a better way of doing it, whether it's too compact, if I should use different data structures, if my comments are too verbose, if my style is alright - basically any kind of tips and advice are very welcome.

from datetime import datetime, timedelta

def process_dates(date):
    """
    Process adjacent days to account for flights that 
    have a different departure day than the day they were
    archived (which is the date contained in the filenames)
    """

    """
    Convert to datetime objects before processing
    This is done so the conversion is less error-prone by 
    avoiding edge cases like 1st of Jan, 29th of Feb, y3k, etc  
    """
    datetimes = []
    today = datetime.strptime(date, '%Y%m%d')

    """
    Compute the adjacent days, but keeping the order of the elements
    the following: today, yesterday, tomorrow, and the day after tomorrow.
    This is done because the looking up later on is done sequentially
    and the order reflects the probability of being the needed date
    (i.e. it's most probable the needed day is today, so it will stop before
    processing the other days)
    """
    [datetimes.append(today + timedelta(days=i)) for i in [0, -1, 1, 2]]
    return [datetime.strftime(j, '%y%m%d') for j in datetimes]
\$\endgroup\$
6
\$\begingroup\$

Yes, the comments are too much. You don't need to explain why you're doing it all; you might not always be using this function for its current purpose. And I wouldn't put general explanatory comments like that inline; put them in the function docstring.

Also, it's not at all Pythonic to do something like:

[datetimes.append(today + timedelta(days=i)) for i in [0, -1, 1, 2]]

filling a list with None then discarding it; either use a list comprehension:

datetimes = [today + timedelta(days=i) for i in [0, -1, 1, 2]]

or append:

 for i in [0, -1, 1, 2]:
     datetimes.append(today + timedelta(days=i))

I would also encapsulate more of the variables in default parameters, to make the function more flexible:

from datetime import datetime, timedelta

def adjacent_dates(date, deltas=[0, -1, 1, 2], format="%Y%m%d"):
    """Generate date strings adjacent to the argument date.

    Note:
      The default order of deltas is to ensure that the most
      likely date is first in the output.

    Args:
      date (str): The date to process.
      deltas (iter of int): The offsets to apply, in days, to the input
        date (defaults to [0, -1, 1, 2]).
      format (str, optional): The format to use for input and output
        (defaults to "%Y%m%d").

    """
    start = datetime.strptime(date, format)
    days = [start + timedelta(days=i) for i in deltas]
    return [d.strftime(format) for d in days]

Now you can use your defaults:

>>> adjacent_dates("20141030")
['20141030', '20141029', '20141031', '20141101']

or easily switch to something else:

>>> adjacent_dates("30-10-2014", range(-2, 3), "%d-%m-%Y")
['28-10-2014', '29-10-2014', '30-10-2014', '31-10-2014', '01-11-2014']

Note that strftime is a datetime instance method, so you can call it on the instances rather than the class - d.strftime(format) is neater than datetime.strftime(d, format).

\$\endgroup\$
  • \$\begingroup\$ @confused00 no problem. You might also consider splitting the functionality for adjacent dates and the to/from string conversion into two separate functions. \$\endgroup\$ – jonrsharpe Oct 30 '14 at 16:43
  • \$\begingroup\$ It would be bad practice to put a mutable object such as a list as a function's default keyword argument. If at any stage it gets mutated every other calls to that function will now be running on a wrong default. Unless you are absolutely sure that no one will touch it, I would strongly recommend you go with something like a tuple. \$\endgroup\$ – Renae Lider Nov 1 '14 at 1:41
  • \$\begingroup\$ @RenaeLider there's no risk here, as that default list is never leaked to the caller (it isn't returned); you could only alter it by modifying func_defaults, in which case a tuple isn't really safe either. \$\endgroup\$ – jonrsharpe Nov 1 '14 at 8:45

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.