5
\$\begingroup\$

I'm writing an Python application that uses frequently datetime and Unix timestamp. I know Python is 'batteries included', however, I found that converting between datetime and timestamp in Python 2.6 is not trivial. That's why I wrote this utility class. This code works as expected, for Python 2.6, 2.7, and 3.4.

I want the code style of my code reviewed. I don't know whether my function names like dttomicrotimestamp and code styles (like @staticmethod) are suitable for pythonic way of coding.

#!usr/bin/python
# -*- coding: utf-8 -*-
from __future__ import absolute_import, print_function, division

import time
import datetime
import pytz
from tzlocal import get_localzone

class datetimeutil:
    @staticmethod
    def strtodt(text):
        try:
            dt = datetime.datetime.strptime(text, "%Y-%m-%d %H:%M:%S.%f")
        except:
            dt = datetime.datetime.strptime(text, "%Y-%m-%d %H:%M:%S")
        return get_localzone().localize(dt)

    @staticmethod
    def dttotimestamp(dt):
        if dt.tzinfo is None:
            dt = get_localzone().localize(dt)
        return datetimeutil._totalseconds(
            (dt - dt.utcoffset()).replace(tzinfo=None) -
            datetime.datetime(1970, 1, 1))

    @staticmethod
    def dttomicrotimestamp(dt):
        return int(datetimeutil.dttotimestamp(dt) * 1e6)

    @staticmethod
    def timestamptodt(timestamp):
        return get_localzone().localize(datetime.datetime.fromtimestamp(timestamp))

    @staticmethod
    def microtimestamptodt(microtimestamp):
        return datetimeutil.timestamptodt(microtimestamp / 1e6)

    @staticmethod
    def _totalseconds(timedelta):
        return ((timedelta.seconds + timedelta.days * 24 * 3600) * 1e6 
                + timedelta.microseconds) / 1e6
\$\endgroup\$
2
\$\begingroup\$

As you apparently suspect already, having a class full of static methods isn't really Pythonic. The only reason to ever do that is for namespacing, but this should all go into a module anyway, so wrapping all the functions in a class is just unnecessary. Flat is better than nested.

You use the abbreviation dt a lot in the function names. Don't. The only time you should use abbreviations like that is if they are more recognisable than spelling out what they stand for (the canonical example is "do use HTTP"). A lot of them probably don't need to mention datetime at all in the name, given they're in a module called dateutil and their only argument is a datetime object.

Similarly, you use the abbreviation "microtimestamp", which is a little opaque. It looks like you mean "Like a Posix timestamp but measured in microseconds", so a more explicit thing to call it is microsecond_timestamp or microsecond_offset.

With those considerations in mind, and with a view to emphasising what the functions do, I would rename them thus:

  • strtodt -> parse_datetime
  • dttotimestamp -> timestamp
  • dttomicrotimestamp -> microsecond_timestamp
  • timestamptodt -> datetime_from_timestamp
  • microtimestamptodt -> datetime_from_microsecond_timestamp

Do the microsecond ones really need to be functions at all? They seem like they'd be used only rarely (probably at the end points, reading and writing to a database), and since they're one trivial calculation, a small comment at the call site would be sufficient.

Some of these really seem like they belong as instance methods on the datetime object. In fact, a lot of them are in newer stdlib, and the entire point of your code is to also support versions of Python from before that was the case. So, it might be a good idea to simply offer a compatible interface. There's a few ways to do that.

First, the stdlib datetime module is implemented in Python, so you could include a newer version of it wholesale and have your dtutil module do this:

if sys.version_info < (2,7,0):
    # Use the newer datetime module even
    # on older versions of Python.
    import datetime34 as datetime
else:
    import datetime

Second, you could inherit from the stdlib one and add the missing functionality. If you want to add _totalseconds directly to timedelta this way as well, you would also have to override __sub__ and __rsub__ to turn any timedelta object they are about to return into your extended timedelta.

I was going to suggest monkey patching as another option (this being one limited context where it does seem worth it), but it turns out you can't (datetime and timedelta use __slots__).

I only recommend adding the strict compatibility things this way. The idea is to set it up so that if you stop supporting 2.6 down the track, you can delete this code without having to adjust anything else, except maybe some imports. parse_datetime in particular ought to stay separate, especially because it fairly specifically enforces/assumes your local policy ("serialised dates will be in one of these two formats").


In this function:

def strtodt(text):
    try:
        dt = datetime.datetime.strptime(text, "%Y-%m-%d %H:%M:%S.%f")
    except:
        dt = datetime.datetime.strptime(text, "%Y-%m-%d %H:%M:%S")
    return get_localzone().localize(dt)

avoid bare except:, it can mask bugs, and even cause some - eg, you might catch and ignore KeyboardInterrupt. except ValueError: instead. Consider adding a docstring to explicitly say that it tries two formats, as well as that it gives you a timezone-aware datetime.


def dttotimestamp(dt):
    if dt.tzinfo is None:
        dt = get_localzone().localize(dt)
    return datetimeutil._totalseconds(
        (dt - dt.utcoffset()).replace(tzinfo=None) -
        datetime.datetime(1970, 1, 1))

The function before this guarantees that it will give an aware datetime, so I think it would be reasonable for this one to assume that is what it is given. Then if you're using the newer version of the module (or faking a compatible interface), then this is just: dt.timestamp().


You import time and localtz, but never use them. Drop those, they're just noise.

\$\endgroup\$
2
\$\begingroup\$

Let's examine the code:

#!usr/bin/python

you don't need to execute the code directly, so why this line?

class datetimeutil:
    @staticmethod

why a class with static methods? Keep it simple, write global functions. You already have the module which encapsulates the functions.

    def strtodt(text):

I don't like too much the names of these functions. I know that also python uses ugly names (words without any separator) but I think you could improve this... for example: str_to_dt or str2dt seem better to me.

About the functionality. Managing dates is a quite difficult task. The python library in my opinion makes a great job in distinguishing naive and aware datetimes. Your wrapper module simplifies this by considering each naive datetime as a localtime (if I understand it correctly). I think it is not good to hide this decision in your code. Explicit is better, even if it could me more verbose.

Also I think you missed the timedelta.total_seconds function, which could be used in your code.

\$\endgroup\$

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.