CLI Twitter client in Python

Question

Here is a simple client for Wwitter that I wrote based on the twitter library in Python. The aim is to be able to read the feeds from different twitter accounts from the same place in a light way.

I should mention that the code does what I want it to do. Nevertheless, how could I improve it?

(It is available on GitHub.)

#!/usr/bin/python

import sys
import os
import getopt
from twitter import *

# Default values
def_counts = 10
users = []
dir_creds = "./data/oauth/"

# input parameters
def usage():
  print "Run: " + sys.argv[0] + " [OPTIONS]"
  print "Where OPTIONS is one the following: "
  print " -h --help"
  print " -c N --counts=N"
  print " -u \"name\" --user=\"name\""
  print " -a --all"
  print " -l --list"

def main(argv):
  global def_counts, users
  try: 
    opts, args = getopt.getopt(argv,"hc:u:al",["help","counts=","user=","all","list"])
  except getopt.GetoptError:
    usage()
    exit(2)
  for opt,arg in opts:
    if opt in ["-h","--help"]:
      usage()
      exit(1)
    elif opt in ["-c","--counts"]:
      print "Retrieving "+str(arg)+" tweets."
      def_counts = arg
    elif opt in ["-u","--user"]:
      print "User: "+arg
      users.append(arg)
    elif opt in ["-a","--all"]:
      print "all users"
      if os.path.exists(os.path.expanduser(dir_creds)):
        token_files = filter(lambda x: x.endswith('.token'), os.listdir(os.path.expanduser(dir_creds)))
        for file in token_files:
          usr = file[:file.index(".token")]
          users.append(usr)
      else:
        print "No user to be added, path undefined"
        exit(2)
    elif opt in ["-l","--list"]:
      if os.path.exists(os.path.expanduser(dir_creds)):
        token_files = filter(lambda x: x.endswith('.token'), os.listdir(os.path.expanduser(dir_creds)))
        for file in token_files:
          usr = file[:file.index(".token")]
          print usr
    else:
      print "Got the following and I don't know what to do with it:"
      print opt + " " + arg
      usage()
      exit(2)


if __name__ == "__main__":
  main(sys.argv[1:])

if len(users) < 1:
  users.append("bilbo_pingouin")

col_bgred   = '\033[41m'
col_bold    = '\033[1m'
col_fgblue  = '\033[34m'
col_fggreen = '\033[32m'
col_fggrey  = '\033[90m'
col_end     = '\033[0m'

for user in users:
  print "\n" + col_bgred + col_bold + user + col_end
  # Retrieve the credentials for a given account
  if not os.path.exists(dir_creds):
    os.makedirs(dir_creds) # I just assume there is not race issue here!

  file_creds = dir_creds + user + ".token"
  MY_TWITTER_CREDS = os.path.expanduser(file_creds)

  api_token_file = "data/api_token.dat"
  if os.path.exists(api_token_file):
    cust_token, cust_secret =     read_token_file(os.path.expanduser(api_token_file))
  else:
    print "ERROR: The app is not identified!"

  if not os.path.exists(MY_TWITTER_CREDS):
    oauth_dance("Twit on CLI",cust_token,cust_secret,
            MY_TWITTER_CREDS)

  oauth_token, oauth_secret = read_token_file(MY_TWITTER_CREDS)

  # OAuth idnetification
  t = Twitter(auth=OAuth(oauth_token,oauth_secret,cust_token,cust_secret))

  # Get status
  data = t.statuses.home_timeline(count=def_counts)

  # Print lines
  for c in range(len(data)):
    print "* " + col_fggreen+col_bold+data[c]['user']['name']+col_end + ' (' + col_fgblue+data[c]['user']['screen_name']+col_end + ')' + " - " + data[c]['text'] + col_fggrey+" ## "+data[c]['created_at']+col_end

Answer 1

For starters using Python 3 or at least being compatible with it would be a good idea. Meaning that all the print calls should be using the function form, i.e. print(...).

Next, take a look at PEP8 and use four spaces for indentation.

Parsing command line arguments is a bit nicer with argparse, but since you're already done with it it might not make too much sense to rewrite that part.

There's duplicated for the "--all" and "--list" cases that would be better served with a common function.

The colour definitions can be taken from some module, e.g. take a look at termcolor or colorama.

Lastly the for c in range(len(data)): will always be a pattern that should be replaced with direct iteration, i.e. for x in data:. The line is also way to long to read it easily.

Answer 2

First things first, let's look at argparsing here.

Your current argument parsing code is... a mess. Something like:

import argparse

def main(argv):
    global def_counts, users
    parser = argparse.ArgumentParser(description='Twitter app')
    parser.add_argument('-c', '--counts', help='Count of tweets to retrieve.', required=True)
    parser.add_argument('-u', '--user', help='Username', required=True)

    parser.add_argument('-a', '--all', help='All Users', required=False)
    parser.add_argument('-l', '--list', help='List', required=True)

Allows you much more flexibility in actually reading your arguments. As you might notice this is... much easier to configure than your current parsing. It is also easier for a person such as myself to read.

This allows you do this for a 'help' option if you have at least 1 required argument and get rid of your usage method:

try:
    options = parser.parse_args()
except:
    parser.print_help()
    sys.exit(0)

If require args are not provided it will throw an exception and print the help (which it will do on -h or -help or --foo-nicorn).

This streamlines your parsing of the args:

def_counts = options['counts']
user = options['user']
users.append(user)

Note that when making strings the following is much more readable:

print "Retrieving {count} tweets.".format(count=def_counts)
print "User: {user}".format(user=user)

Especially when you have longer strings, such as at the end of your code, using format and the variables as parameters is much more clear.

You might notice the parsing for list and all are almost identical, with the exception of printing or appending. I rewrote this, using your options again (note that an unprovided arg to argparse is None so you can do if arg: for checking the argument was provided):

if options['all'] or options['list']:
    print "all users"
    if os.path.exists(os.path.expanduser(dir_creds)):
        token_files = filter(lambda x: x.endswith('.token'), os.listdir(os.path.expanduser(dir_creds)))
        for f in token_files:
            usr = f[:f.index(".token")]
            if options['all']:
                users.append(usr)
            else:
                print usr     
    else:
        print "No user to be added, path undefined"
        exit(2)

I also changed file to f as file is a keyword in Python.

This removes a fair bit of duplication and also makes reading your logic much less painful.

Everything here should reproduce how your logic parsing your args is currently (full disclaimer: it's Friday night and... no guarantees :-).

For pep8/flake8 you generally want to add spaces after commas, too.