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 i in range(height)
for t in text.split('\n')
])