Shift\Transform characters in a grid

Question

I wrote this program for a code test on some website. This program do some predefined transformations (transformation can be horizontally, vertically or by shifting to given number of keys) for every char in a file. For every char same set of transformations should be done. After doing all the transformations the new char on the position of input char should be displayed. The file size can go in GB's

How can I improve the efficiency of this program? I only intend to change the Python language usage not the algorithm.

import datetime

def h_transform(grid):
    '''
    function to transform grid horizontally i.e. 5th column will be swapped by
    6th column and 4th col will be swapped by 7th col .... 1st col with 10th col
    '''
    for row in range(len(grid)):
        number_fo_keys = len(grid[row]) - 1
        #iterate only upto half of cols
        for col in range(number_fo_keys/2):
            #swap the values
            temp = grid[row][col]
            grid[row][col] = grid[row][number_fo_keys - col]
            grid[row][number_fo_keys - col] = grid[row][col]

def v_transform(grid):
    '''
    function to transform grid vertically i.e. 2nd row will be swapped by
    3rd row and 1st row will be swapped by 4th row
    '''
    #iterate only upto half of rows
    number_fo_rows = len(grid) - 1
    for row in range(number_fo_rows/2 + 1):
        for col in range(len(grid[row])):
            #swap the values
            temp = grid[row][col]
            grid[row][col] = grid[number_fo_rows - row][col]
            grid[number_fo_rows - row][col] = grid[row][col]

def s_transform(grid, shift_by):
    '''
    function to shift grid by number of keys. number can be +ve (right shift)
    or -ve (left shift)
    '''
    number_fo_rows = len(grid) - 1
    for row in range(number_fo_rows + 1):
        shifted_list = []
        number_of_keys = len(grid[row])
        #positive shift
        if shift_by > 0:
            if row == number_fo_rows:
                next_row = 0
            else:
                next_row = row + 1
            shifted_list = grid[row][number_of_keys - shift_by:]
            grid[row] = grid[row][:number_of_keys - shift_by]
            grid[next_row] = shifted_list + grid[next_row]
        else:
            if row == 0:
                next_row = number_fo_rows
            else:
                next_row = row - 1
            shifted_list = grid[row][:abs(shift_by)]
            grid[row] = grid[row][abs(shift_by):]
            grid[next_row] = grid[next_row] + shifted_list

if __name__ == '__main__':
    start_date = datetime.datetime.now()
    print start_date
    grid = [['1','2','3','4','5','6','7','8','9','0'],
            ['q','w','e','r','t','y','u','i','o','p'],
            ['a','s','d','f','g','h','j','k','l',';'],
            ['z','x','c','v','b','n','m',',','.','/']]
    transformations = ['H', 5, 'V', -1]

    #creating a lookup dictionary for grid data
    grid_index_dict = {}
    for row in range(len(grid)):
        for col in range(len(grid[row])):
            grid_index_dict.update({grid[row][col]:[row, col]})

    with open("keyboard_data.txt", "r") as f:
        for char in f.read():
            new_grid = list(grid)
            row_index = None
            col_index = None
            #getting the row and column index for input char
            index_list = grid_index_dict.get(char, None)
            if index_list:
                for tranformation in transformations:
                    if tranformation == 'H':
                        h_transform(new_grid)
                    elif tranformation == 'V':
                        v_transform(new_grid)
                    else:
                        s_transform(new_grid, tranformation)
                if new_grid:
                    print "Original Letter: ", char, 
                    print "Tranformed Letter: ", new_grid[index_list[0]][index_list[1]]
            else:
                print char

    print "Process complete"
    end_date = datetime.datetime.now()
    print end_date
    print end_date - start_date

Sample content of the "keyboard.txt" file:

qw

Output should be with transformation's 'H',5,'V',-1 will be:

ui

I don't have the original problem statement, which is why I am looking for comments to improve usage of Python constructs in my code and not the algorithm.

Answer 1

As you haven't presented the problem statement it's hard to say what the best approach would be - for example, it seems odd that you apply the transformations between each character, rather than once at the start. However, here are a few comments independent from the overall approach.

---

You have an awful lot of code in your if __name__ == "__main__": block. You should really split it out into more functions, for example:

if __name__ == "__main__":
    start_date = datetime.datetime.now()
    print start_date

    grid = create_grid()
    grid_index = create_index(grid)
    transformations = ['H', 5, 'V', -1]
    process_file("keyboard_data.txt", grid, transformations)

    print "Process complete"
    end_date = datetime.datetime.now()
    print end_date
    print end_date - start_date

---

new_grid = list(grid)

will only create a shallow copy of grid - new_grid contains references to the same list objects within grid.

---

Dictionaries can simplify long if blocks:

funcs = {'H': h_transform, 'V': v_transform}

...

for transformation in transformations:
    try:
        funcs[transformation](new_grid)
    except KeyError:
        s_transform(new_grid, transformation)

---

The following two lines seem totally pointless:

row_index = None
col_index = None

---

It is better to use string formatting than concatenation, for example:

print "Original Letter: {0}".format(char),

---

for index in range(len(seq)):

is rarely the Pythonic approach to take. For example, you could use:

for index, item in enumerate(seq):

to access the item and its index simultaneously.

---

A temporary swap variable isn't very Pythonic either, you can simplify this with tuple packing and unpacking:

>>> a = 1
>>> b = 2
>>> a, b = b, a
>>> a
2
>>> b
1

---

h_transform and v_transform are broken, for example:

>>> grid = [[1, 2, 3, 4], [1, 2, 3, 4]]
>>> h_transform(grid)
>>> grid
[[4, 2, 3, 4], [4, 2, 3, 4]]

Given that you are just reversing each row, why not:

for row, index in enumerate(grid):
    grid[index] = row[::-1]

---

It's not clear why you have the offset

number_fo_rows = len(grid) - 1

then add it back in the next line:

for row in range(number_fo_rows + 1):

---

You could consider an OOP approach to hold the state of the grid and the transformations together:

def Grid(object):

    def __init__(self):
        self.grid = ...

    def transform(self, transformations):
        ...

    def process_char(self, char):
        ...

    def _h_transform(self):
        ...

    def _s_transform(self, shift):
        ...

    def _v_transform(self):
        ...

Then you can do something like:

grid = Grid()

grid.transform(transformations)

for char in ...:
    grid.process_char(char)