Image labeling: it's Monday again

This command line program applies defined labels along with Monday's date to a directory of images.

The script has to handle not being run on a Monday. Others are asking for the script so I cleaned it up as much as I could. I want to make it as friendly and as cross-platform as possible.

Labels should be applied just above where the white border region ends. I couldn't guarantee that the border region would always be white, nor how big the border region could be.

Any tips on making it shorter, more elegant, more user-friendly or more resilient would be helpful. I'm considering moving the conditions and text of the actual labels to a separate text file for more flexibility.

```
import os
import datetime
from datetime import timedelta
import argparse
import glob
import PIL
from PIL import Image, ImageFont, ImageDraw

path_sep = os.path.sep

def get_date_string(optionalArg):
#return date of most recent Monday
# or optional date param given by user.
if optionalArg is not None:
return optionalArg
else:
today = datetime.datetime.today()
if today.weekday() == 0:
t = today
else:
t = today - timedelta(days=today.weekday())
return t.strftime("%m/%d/%Y")

def significant_difference(v, previous_v):
return abs(sum(v) - sum(previous_v)) > 200

def find_v_border(png):
h = png.height
png_ims = png.load()
x = png.width/2
last_v = png_ims[x,h-1]
# walk up image from bottom, one pixel at a time. when color changes, identify that as the place to write text:
for y in range(h-1,0,-1):
v = png_ims[x,y]
if significant_difference(v,last_v):
return y
last_v = v

def find_left_border(png,bottom_border):
png_ims = png.load()
last_v = png_ims[1,bottom_border]
for x in range(1,png.width/2):
v = png_ims[x,bottom_border]
# when big difference between v and last v, then return x
if significant_difference(v,la

Top level docstring

It is good practice to start a file with a string delimited by triple quotes that explains the general purpose of your file.

Python treats it specially and displays it when queried for help(your_module) and saves it in the __doc__ special variable.

Other people will get to know the general purpose of the code without reading it all.

A very basic start would be:

"""
This script adds time-stamps to images,
with particular regards to Mondays.
"""

Confusing parameter name

You write def get_date_string(optionalArg): where optionalArg is not actually optional... I am forced to provide it, an optional argument is one like def foo(a=1):

When names contrast the code I become very confused, also I do not think that kind of returning if not None is so useful, but I am not sure...

Actually you only call the function from:

image_date = get_date_string(args.d)

So you can remove the argument and the None check and call it like:

image_date = args.d or get_date_string()

Or, the equivalent:

image_date = args.d if args.d is not None else get_date_string()

Reduce nesting

else:
    today = datetime.datetime.today()
    if today.weekday() == 0:
        t = today
    else:
        t = today - timedelta(days=today.weekday())
    return t.strftime("%m/%d/%Y")

Becomes:

today = datetime.datetime.today()
if today.weekday() == 0:
    t = today
else:
    t = today - timedelta(days=today.weekday())
return t.strftime("%m/%d/%Y")

Because you already returned, and when you return the function ends, so you can reduce nesting and make it clearer that the None check is just a precondition.

Inline ternary

I like ternaries because they are less powerful than conditionals, being less powerful means that they are easier to read and understand too:

if today.weekday() == 0:
    t = today
else:
    t = today - timedelta(days=today.weekday())

Becomes:

t = today if today.weekday() == 0 else today - timedelta(days=today.weekday())

Monday is equal to zero, but you should not care

You write today.weekday() == 0 to check if weekday is Monday, but it is much more intuitive to read: today.weekday() == MONDAY

To allow this just add:

MONDAY = 0

at the top of your code, and you gained a lot of readability with a small effort.

Avoid single letter names

t tells me nothing about the purpose of this variable, try and think about a more descriptive name.

200 it was said...

def significant_difference(v, previous_v):
    return abs(sum(v) - sum(previous_v)) > 200

Why 200? This is very confusing, make a constant containing that value and give an explanatory name to it, because a random magic number 200 in the code just leaves me wondering...

Similar ...

The function find_v_border and find_left_border are similar, but sadly I cannot think of a way to abstract the similarity out... I just wanted to point this out as a possible further modularization possibility.

Not needed global

global is needed only when modifying global variables, so you may delete it from draw_on_image.

Re-use

Re-using singular functions is impossible now, as all the script will get executed if I import it, just putting the top-level code under a if __name__ == "__main__": will allow the re-use of single functions.

Function for deciding name and content

This block of code:

if "server" in str.lower(file_name):
    text = " Server Systems"
    new_file_name =  "labelled_server.png"
elif "desktop" in str.lower(file_name):
    text = " Desktop Desktop"
    new_file_name = "labelled_Desktop.png"
else:
    text = " All Systems"
    new_file_name = "labelled_all.png"

Only does one thing, so extracting it to a function looks like a good idea:

def find_text_and_new_name(file_name):
    if "server" in str.lower(file_name):
        return " Server Systems", "labelled_server.png"
    elif "desktop" in str.lower(file_name):
        return " Desktop Desktop", "labelled_Desktop.png"
    else:
        return " All Systems", "labelled_all.png"

Calling this from the top-level makes it more abstract, and you always want your top-level code to be as abstract as possible.

Increase generality

The draw_on_image function may easily be customized by the use of optional arguments, for example why should font always be 16? Making it an optional argument increases generality thus utility of your function.

(minor) Avoid unnecessary assignment

os.path.sep is never going to change, so you do not need to store it in another variable.

StackExchange Code Review Q#114394, answer score: 4