Small Twitter bot made in Python

Question

I've just started to play around with Python and I've decided to create a little script that post tweets every hour (cronjob on Ubuntu) telling what time it is.

I'm not sure if I follow best practices or if I can make the code even shorter/cleaner/nicer? I'm a C# developer so I miss a few things in Python, like string.Empty and list.First(). Do these features exist in Python?

from twitter import *
from datetime import datetime
import pytz

TOKEN = "SECRET_STUFF"
TOKEN_KEY = "SECRET_STUFF"
API_KEY = "SECRET_STUFF"
API_SECRET = "SECRET_STUFF"

def init(timestamp = datetime.now(pytz.timezone('Europe/Stockholm')).time()):
    number_of_rings = get_number_of_times_to_display_alarm(timestamp)
    send_tweet(compose_tweet(number_of_rings, timestamp))

def get_number_of_times_to_display_alarm(timestamp):
    # Convert our timestamp to 12h format, split it, select first element(hours)
    # and cast to int to remove leading 0 (e.g. 09.00 will give us 9)
    return int(timestamp.strftime("%I:%M %p").split(':')[0])

def compose_tweet(number_of_rings, timestamp):
    # Creates a tweet, e.g. "BONG...BONG...BONG #03:00
    ALERT_SOUND = "BONG"
    tweet = ""
    for i in range(number_of_rings):
        tweet += "%s%s" %(ALERT_SOUND, "..." if i < (number_of_rings - 1) else "")

    tweet += " #%s:%s" %(str(timestamp.hour).zfill(2), str(timestamp.minute).zfill(2))
    return tweet

def send_tweet(tweet):
    auth = OAuth(TOKEN, TOKEN_KEY, API_KEY, API_SECRET)
    t = Twitter(auth=auth)
    t.statuses.update(status=tweet)

init()

UPDATE Based on the great feedback I got from @LiavK I've now updated my code as follows:

from twitter import OAuth, Twitter
from datetime import datetime
import pytz

TOKEN = "SECRET_STUFF"
TOKEN_KEY = "SECRET_STUFF"
API_KEY = "SECRET_STUFF"
API_SECRET = "SECRET_STUFF"

MY_TZ = pytz.timezone('Europe/Stockholm')
now = datetime.now(MY_TZ).time()

def get_current_hour(timestamp):
    return timestamp.hour % 12

def compose_tweet(number_of_rings, timestamp):
    # Creates a tweet, e.g. "BONG...BONG...BONG #03:00
    ALERT_SOUND = "BONG..."
    tweet = (ALERT_SOUND * number_of_rings)[:-3] # Removes last ...
    tweet += " #%s:%s" %(str(timestamp.hour).zfill(2), str(timestamp.minute).zfill(2))
    return tweet

def send_tweet(tweet):
    auth = OAuth(TOKEN, TOKEN_KEY, API_KEY, API_SECRET)
    t = Twitter(auth=auth)
    t.statuses.update(status=tweet)

def main(timestamp = now):
    number_of_rings = get_current_hour(timestamp)
    send_tweet(compose_tweet(number_of_rings, timestamp))

if __name__ == "__main__":
    main()

Answer 1

from twitter import * 

Importing * is generally considered bad practice, because you don't know what's being imported into your local namespace. For you case, I believe you can get away with just doing import twitter. In the general case, if you only need a few things from a module, you can do:

from foo import bar, baz, quux

Alternatively if you need everything from foo just do import foo and then call foo.bar(), foo.baz(), etc. If you get tired of typing long names, you can do something like import jabberwocky as foo.

def init(timestamp = datetime.now(pytz.timezone('Europe/Stockholm')).time()):

I think there's serious readability issues with this, and I also can't think of any time I've ever seen a function/method being called inside another function's signature. I'm going to go out on a limb and say this is a Bad Idea -- for one thing, I suspect that automated code documentation tools might have trouble parsing this. More fundamentally, a function/method's signature is its interface with the rest of the world. It just seems like a very bad trade off to muddy the readability of a signature in exchange for saving a couple of times of code. I would refactor to something like:

MY_TZ = pytz.timezone('Europe/Stockholm')
now = datetime.now(MY_TZ).time()
def init(timestamp = now):

You could also think about how you actually initialize this script. You mentioned you're running it as a cron job, so I think this layout makes sense, but you could also rewrite this as a class and place the init logic into the __init__ method of the class.

def get_number_of_times_to_display_alarm(timestamp):
    # Convert our timestamp to 12h format, split it, select first element(hours)
    # and cast to int to remove leading 0 (e.g. 09.00 will give us 9)
    return int(timestamp.strftime("%I:%M %p").split(':')[0])

This is not bad at all, but here are a couple of alternatives:

now = datetime.now(MY_TZ).hours
if now > 12:
    now -= 12
return now 

We've already used datetime.now(), it's both simpler than timestamp.strftime and it's nice to not have to introduce new functions unless necessary. There's an even shorter alternative you could use, which trades off a little readability for space:

now = datetime.now(MY_TZ).hours % 12

This uses the modulo function to convert 24h to 12 h clock.

Also, it's better to use docstring in for your code documentation. Something like "Convert our timestamp to 12h format, split it, select first element(hours) and cast to int to remove leading 0 (e.g. 09.00 will give us 9)" is closer to being documentation for yourself/others. It should be a docstring. You should also think about removing the implementation specific elements -- those are more appropriate to use a # comment. I would rewrite the comment as:

"""Convert our timestamp to 12h format and remove leading zeros. Returns an Int."""

That decouples the interface of the function from its implementation.

One nitpick about your above code, which I only noticed noodling around with it now in a shell session: get_number_of_times_to_display_alarm. That's a very long function name. It's generally good to be longer than shorter and more explicit than less, but also consider it can be tiring and people have to remember the names of your functions. Try to find the shortest name that accurately and specifically describes what the logic does. I would probably name that logic something like get_current_hour.

def compose_tweet(number_of_rings, timestamp):
    # Creates a tweet, e.g. "BONG...BONG...BONG #03:00
    ALERT_SOUND = "BONG"
    tweet = ""
    for i in range(number_of_rings):
        tweet += "%s%s" %(ALERT_SOUND, "..." if i < (number_of_rings - 1) else "")

This could be sweated down a bit. For example:

for i in range(hour):
    tweet += "%s%s" % (ALERT_SOUND, "..." if i < (hour - 1) else "")

I don't think you need the else clause in that. You could refactor it to something like:

def compose_tweet(hour, timestamp):
    """ Creates a tweet, e.g. "BONG...BONG...BONG #03:00. """
    ALERT_SOUND = "BONG..."
    tweet = ""
    for i in range(hour):
        tweet += ALERT_SOUND   
    tweet += " #%s:%s" % (str(timestamp.hour).zfill(2), str(timestamp.minute).zfill(2)) 
    return tweet 

An even shorter version could be built around tweet = ALERT_SOUND * hour.

init()

A common pattern for python modules is to place:

if __name__ == "__main__":
    init()

At the end of the file. This allows the file to either run as a script or to be imported into other modules. For example, if you wanted to reuse your get_current_hour function in a different project, you could then do from twitClock import get_current_hour.

Answer 2

Some notes on your second code:

• MY_TZ = pytz.timezone('Europe/Stockholm') + now = datetime.now(MY_TZ).time(): I don't see why this is executed in the module load and not within main.

• ALERT_SOUND = "BONG...". No capitalized variable names as local variables.

• (ALERT_SOUND * number_of_rings)[:-3] -> "...".join([ALERT_SOUND_WITHOUT_DOTS] * number_of_rings)

• tweet += Code is clearer without in-place updates. Create a list and join or concatenate strings with different names.

• t = Twitter(auth=auth). What is t? time? if you call it twitter, nobody will doubt what is is.

• number_of_rings = get_current_hour(timestamp): compose_tweet has timestamp as argument, why don't you calculate the number of rings there?