Python ASCII-Art Text Generator

Question

 _____ _____ _____ _____ _____ _______ /\ / ____| / ____| |_ _| |_ _| /\ | __ \ |__ __| / \ | (___ | | | | | | / \ | |__) | | | / /\ \ \___ \ | | | | | | / /\ \ | _ / | | / ____ \ ____) | | |____ _| |_ _| |_ / ____ \ | | \ \ | | /_/ \_\ |_____/ \_____| |_____| |_____| /_/ \_\ |_| \_\ |_| 

---

As a part of a console utilities module that I'm building, the central function (after which the module is named) is a function that generates ASCII art representation of given text.

Any comments or suggestions for improvement will be great!

default_conf = open('C://Programs//Python35//Lib//asciiart.txt').read() ... def asciiart (text, conf=default_conf): """Produces an ascii art representation of text using the conf data""" if isinstance(conf, str): conf = conf.split('\n') height, key, data = int(conf[0]), conf[1], conf[2:] figures = dict(map(lambda x: (x, data[key.index(x) * height : key.index(x) * height + height]), key)) art = '\n'.join(['\n'.join([' '.join([figures[x][i] for x in t.lower() if x in figures]) for i in range(height)]) for t in text.split('\n')]) return art 

Here is a demo:

>>> import asciiart >>> print(asciiart.asciiart('code-review')) _____ ____ _____ ______ _____ ______ __ __ _____ ______ __ __ / ____| / __ \ | __ \ | ____| | __ \ | ____| \ \ / / |_ _| | ____| \ \ / / | | | | | | | | | | | |__ ____ | |__) | | |__ \ \ / / | | | |__ \ \ /\ / / | | | | | | | | | | | __| |____| | _ / | __| \ \/ / | | | __| \ \/ \/ / | |____ | |__| | | |__| | | |____ | | \ \ | |____ \ / _| |_ | |____ \ /\ / \_____| \____/ |_____/ |______| |_| \_\ |______| \/ |_____| |______| \/ \/ 

---

The configuration file format is as follows:

• First line: the height of every character in the configuration file (number of lines).

• Second line: the key. A string containing the letters in the order they appear in the configuration file.

• Next height * len(key) lines: the text representation of the characters.

The full configuration file can be found here (covers most of the printable ASCII characters).

Answer 1

Style

You’ve already been told that by @alexwlchan in your previous question but: "whitespace is cheap" and it improves readability a lot. Compare your code to:

def asciiart(text, conf=default_conf): """Produces an ascii art representation of text using the conf data""" if isinstance(conf, str): conf = conf.split('\n') height, key, data = int(conf[0]), conf[1], conf[2:] figures = dict(map( lambda x: (x, data[key.index(x) * height: (key.index(x) + 1) * height]), key)) art = '\n'.join([ '\n'.join([ ' '.join([figures[x][i] for x in t.lower() if x in figures]) for i in range(height)]) for t in text.split('\n')]) return art 

Again, you should also consider using generator expressions rather than list-comprehensions to feed into your join calls: turn those square brackets into parenthesis.

Handling font configuration

First of, when reading the file, you should remember to close it:

with open('C://Programs//Python35//Lib//asciiart.txt') as configuration_file: default_conf = configuration_file.read() 

You may also want to name the resulting string DEFAULT_CONF as it is a constant.

Second, why parse this string at each call of asciiart? You should provide a function that will do the parsing and asciiart will only be responsible of using the parsed configuration:

ASCII_ART_CONFIGURATION = {} def parse_ascii_art_configuration(filename='C://Programs//Python35//Lib//asciiart.txt'): with open(filename) as f: conf = f.read().split('\n') height, key, data = int(conf[0]), conf[1], conf[2:] figures = dict(map( lambda x: (x, data[key.index(x) * height: (key.index(x) + 1) * height]), key)) ASCII_ART_CONFIGURATION.update(figures) def asciiart(text): if not ASCII_ART_CONFIGURATION: parse_ascii_art_configuration() art = '\n'.join( '\n'.join( ' '.join(ASCII_ART_CONFIGURATION[x][i] for x in t.lower() if x in ASCII_ART_CONFIGURATION) for i in range(height)) for t in text.split('\n')) return art 

The advantage being:

1. You give your users direct access to the underlying ASCII_ART_CONFIGURATION that they can populate the way they want if need be;
2. You give your users an easy way to provide their own configuration filepaths;
3. You parse the configuration only once.

Obviously, this version wont work since asciiart does not have access to height any more. Time to bring our old friend zip (or itertools.zip_longest) back:

from itertools import zip_longest def asciiart(text): if not ASCII_ART_CONFIGURATION: parse_ascii_art_configuration() art = '\n'.join( '\n'.join( ' '.join(row) for row in zip_longest( *(ASCII_ART_CONFIGURATION[x] for x in t.lower() if x in ASCII_ART_CONFIGURATION), fillvalue='')) for t in text.split('\n')) return art 

Better iterations

Now that we have a better API, let's focus a bit more on the code:

• iteration over a file can be made using either the "reading" API or the iterator one, this can help better handle the separation of each part:

   with open(filename) as f: height = int(next(f)) keys = next(f) data = [line.rstrip('\n') for line in f] 

• building of the figure dictionary can be made more efficient using the grouper recipe from itertools:

   def grouper(iterable, n): "Collect data into fixed-length chunks or blocks" # grouper('ABCDEFG', 3) --> ABC DEF G" args = [iter(iterable)] * n return zip_longest(*args, fillvalue='') def parse_ascii_art_configuration(filename='C://Programs//Python35//Lib//asciiart.txt'): with open(filename) as f: height = int(next(f)) keys = next(f) data = [line.rstrip('\n') for line in f] figures = { character: list(group) for character, group in zip(keys, grouper(data, height)) } ASCII_ART_CONFIGURATION.update(figures) 

• now that we use zip_longest to concatenate ascii-art letters, you can simplify handling missing symbols by turning ASCII_ART_CONFIGURATION into a collections.defaultdict:

   ASCII_ART_CONFIGURATION = defaultdict(list) def asciiart(text): if not ASCII_ART_CONFIGURATION: parse_ascii_art_configuration() return '\n'.join( '\n'.join( ' '.join(row) for row in zip_longest( *(ASCII_ART_CONFIGURATION[x] for x in line_of_text), fillvalue='�')) for line_of_text in text.lower().splitlines()) 

Answer 2

Ways you can improve your code:

• The body of the if should be on a new line to the if. Which makes it much more readable.

• Dictionary comprehensions are easier to read than dict(map()). Incase you didn't know Pythons BDFL Guido Van Rossum has had plans to remove lambda, reduce, filter and map from Python. And so I can't encourage the use of map or lambda, even if they're still in Python3.

• Your comprehensions are hard to read. Don't put them all on one line! Spread them out.
  Take:

  art = '\n'.join(['\n'.join([' '.join([figures[x][i] for x in t.lower() if x in figures]) for i in range(height)]) for t in text.split('\n')]) 

  And:

  art = '\n'.join([ '\n'.join([ ' '.join([ figures[x][i] for x in t.lower() if x in figures ]) for i in range(height) ]) for t in text.split('\n') ]) 

  I can read the second much better than the first. I also don't know anyone that would object to this.

• I find assigning to a variable to then return it quite redundant.

Ways you could improve your design:

• I find the creation of figures to be quite hard to read. What you're doing is making a dictionary of data in groups/chunks of height. Instead of what you're doing now, you could instead use the itertools recipe for grouper. And zip the result with the keys.

• Your could add a check to see if the data is divisible by the height. This is a basic check to see if the data is actually correct. As when I was checking my changes, I didn't use a raw multi-line string. Which this check would warn about.

• You can merge the outer two '\n'.join comprehensions into one. This is kind of hard to explain, but since you're making a list of list of strings. And you're performing the '\n'.join on both the list of string, and the list of list of strings. To get a string, you can just do it on a list of strings.

This can result in something like:

from itertools import zip_longest def grouper(iterable, n, fillvalue=None): "Collect data into fixed-length chunks or blocks" # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx" args = [iter(iterable)] * n return zip_longest(*args, fillvalue=fillvalue) def asciiart(text, conf=default_conf): if isinstance(conf, str): conf = conf.split('\n') height, keys, data = int(conf[0]), conf[1], conf[2:] if len(data) % height: raise ValueError("Config's data is not divisable by it's height.") figures = dict(zip(keys, grouper(data, height))) return '\n'.join([ ' '.join([ figures[x][i] for x in t.lower() if x in figures ]) for t in text.split('\n') for i in range(height) ])