File compressor

Question

I have written a file compressor in python. The program generates a file filled with sequences of the same character with a random length, and compresses that file. I would like feedback on code readability, efficiency, and any other nitpicks that you find. Any and all feedback is considered and appreciated!

compress.py

""" Import Statements """
import os
import random
import re
import string

UNCOMPRESSED_FILE = "code/python/compression/uncompressed.txt"
COMPRESSED_FILE = "code/python/compression/compressed.txt"
RE_UNCOMPRESSED_FILE = "code/python/compression/re_uncompressed.txt"

def get_random_letter(previous_letter):
    """ Gets a letter that isn't the previous letter """
    letter = random.choice(string.ascii_lowercase)
    return letter if letter != previous_letter else get_random_letter(previous_letter)

def generate_file():
    """ Generates file to be compressed """
    previous_letter = "A"
    with open(UNCOMPRESSED_FILE, "w") as file:
        for _ in range(random.randint(10000, 100000)):
            letter = get_random_letter(previous_letter)
            for _ in range(random.randint(3, 9)):
                file.write(letter)
            previous_letter = letter

def compress_file(file_to_be_compressed):
    """ Creates a new compressed file from the past file """
    compressed_string = ""
    characters = file_to_list(file_to_be_compressed)
    count = 0
    for i in range(len(characters)):
        try:
            if characters[i] == characters[i + 1]:
                count += 1
            else:
                compressed_string += f"{characters[i]}{count + 1}"
                count = 0
        except IndexError:
            if characters[len(characters) - 1] == characters[len(characters) - 2]:
                count += 1
                compressed_string += f"{characters[i]}{count}"

    with open(COMPRESSED_FILE, "w") as compressed_file:
        compressed_file.write(compressed_string)

def decompress_file(file_to_be_decompressed):
    """ Decompresses passed file """
    compressed_content = file_to_one_line(file_to_be_decompressed)
    decompressed_content = re.findall("..", compressed_content)
    #print(decompressed_content)
    with open(RE_UNCOMPRESSED_FILE, "w") as re_uncompressed_file:
        for pair in decompressed_content:
            letter = pair[0]
            num = int(pair[1])
            re_uncompressed_file.write(letter * num)

def get_file_size(file):
    """ Returns the file size, in bytes """
    return os.path.getsize(file)

def file_to_one_line(file):
    """ Concats all lines in file to one line """
    with open(file, "r") as file_to_read:
        lines = ""
        for line in file_to_read:
            lines += line
        return line

def file_to_list(file):
    """ Converts all the lines into an array of characters"""
    with open(file, "r") as file_to_read:
        lines = ""
        for line in file_to_read:
            lines += line
        return list(map(str, lines))

if __name__ == '__main__':
    generate_file()
    print(f"File size before compression: {get_file_size(UNCOMPRESSED_FILE)}")
    compress_file(UNCOMPRESSED_FILE)
    print(f"File size after compression: {get_file_size(COMPRESSED_FILE)}")
    decompress_file(COMPRESSED_FILE)
    print(f"File size after decompression: {get_file_size(RE_UNCOMPRESSED_FILE)}")

Answer 1

DocStrings

"""Docstrings""" are not comments. They are extracted by the help() function to return documentation to the user on how to use the module. As written, if someone was to:

>>> import compress
>>> help(compress)

They would be given the description of the module as:

Import Statements

Use comments lines # ... for descriptions about the code, and """Doc-strings""" for descriptions of how to use the module/functions.

---

Don't use Recursion for Simple Loops

def get_random_letter(previous_letter):
    letter = random.choice(string.ascii_lowercase)
    return letter if letter != previous_letter else get_random_letter(previous_letter)

Here, you want to generate random lowercase letter which is different from the previous letter. If Python used Tail Call Optimization, this would be fine; the Python compiler would turn the recursive call into a jump statement to the top of the function. But it does (and can't, due to the dynamic interpretive nature). So each recursive call adds another stack-frame. With a 1-in-26 possibility of getting a duplicate letter, the expected stack-frame depth will never get very large, but it is still the wrong control structure. Just use a while loop.

def get_random_letter(previous_letter):
    letter = previous_letter             # To ensure loop executes at least once
    while letter == previous_letter:
        letter = random.choice(string.ascii_lowercase)
    return letter

---

O(1) memory

Compressing a 1,000,0000,000 character file will require over 32,000,000,000 bytes of memory, which seems a bit excessive.

• the file is read into memory as a long string
• the long string is split into a list of single character strings.
• each character is an object of at least 28 bytes
• each entry in the list is at 4 or 8 byte pointer to the corresponding object.

Yeouch!

Instead of reading the entire file into memory, and then splitting it, and then writing out the compressed information:

• open the input & output files simultaneously
• read in characters one at a time
• increment a count when the character is the same as the last
• write out the character and count when the next character is different.

Active memory usage: 2 characters and 1 integer.

---

A string is a list of characters

Well, no, a string is a sequence of characters. But the upshot is the same. If you want to index a string character by character, just index the string. There is no need to turn it into a list:

s = "HELLO"
s[4] == "O"              # The easy way

l = list(map(str, s))    # Or the hard, and
l[4] == "O"              # memory inefficient way

---

Bug / deficiency

Your generate_file() method avoids generating a sequence of more than 9 entries of the same character, but a compression function should be able to handle that, if it happens.

At present, if you compressed AAAAAAAAAAAAAAABB, it would compress to A15B2, which would be decompressed as A1, a single A, followed by 5B which is B occurrences of the the letter 5 ... which will crash the decompression algorithm.

When your compressing encounters the 9th A, it should emit A9 and then reset the count to zero, so AAAAAAAAAAAAAAABB would compress at A9A6B2.

Or your decompression algorithm should look for all of the digits after the letter, and interpret it as a multi digit number.

---

DRY: Don't Repeat Yourself

file_to_one_line and file_to_list contain the same code. You could write one in terms of the other:

def file_to_list(file):
    return list(map(str, file_to_line(file)))

... not that I like this function.

---

Read the Python Docs

You are doing a lot of work to read the file line by line and accumulate the result as a string, when Python allows you to read in the entire file as a string:

def file_to_one_line(file):
    """ Concats all lines in file to one line """
    with open(file, "r") as file_to_read:
        return file_to_read.read()