French animal generator

I'm learning Python. I'm making a random animal generator, from 5 txt files. Each file is a list of words for respectively the specie of the animal, an adjective, an origin, and a bodypart with a color. Not all may appear in the description. I also wanted it to be arithmogrammatic. Here is my code, I know it can be improved, but how ?

import time
import random
z=5

for w in range (120):
z=random.randint(1,100)
Inom=[x for x in nom if len (x) == z ]
Iori=[x for x in ori if len (x) == z ]
Icor=[x for x in cor if len (x) == z-2 ]
Icou=[x for x in cou if len (x) == z ]
try:
name = random.choice(Inom)
orig = random.choice(Iori)
corp = random.choice(Icor)
coul = random.choice(Icou)
except IndexError :
continue
r = random.randint(1,5)
print ""
if r == 0:
print name
elif r == 1:
print name
print orig
elif r == 2:
print name
print "a " + corp
print coul
elif r == 3:
print name
print "a " + corp
print coul
elif r == 4:
print name
print orig
print "a " + corp
print coul
elif r == 5:
print name
print orig
print "a " + corp
print coul


The quite chaotic results look like this :

gecko
d'ete

merles
musque
d'Iran
a tete
Prasin

moucheron
d'Espagne

foulque
pelerin
d'hiver
a duvet
Abricot

crocodile
a griffes
Malachite

sphecides
de bassan
a remiges
Framboise

okapi
a pic
Safre

gasteruption
constricteur
d'Angleterre
a couverture
Vert poireau

chardonneret
garde-boeufs
a couverture
Cafe au lait

vireos
a tete
Tomate

calligraphe
du Pakistan
a mandibule
Bleu marine

enrouleuses
de Finlande
a mandibule
Bleu canard

puceron
d'arbre
a joues
obscure

hemipteres
a criniere
Anthracite

perroquet
des tours
a remiges
Amethyste

porte-musc
a poitrine
Vert gazon

mamba
a pic
blanc

chaus
huppe
a pic
brune

serpent
de Bali
a duvet
Abricot

loriot
d'Inde

etoiledemer
flamboyante
de Brewster
a mandibule
Vert meleze

bronze
maçon
de mer
a aile
Soufre

hesperie
d'egypte

grande
d'Irak
a lore
Brique

belette
a duvet
Abricot

cochond'inde
d'Angleterre
a scapulaire
Vert prairie

moucheron
atricille
de Russie
a collier
Amethyste

salamandre
parenthese
a poitrine
Heliotrope

dendrocygne
de Birmanie
a mandibule
Bleu Persan

tephritides
a mandibule
Tourterelle

trichopteres
a scapulaire
Jaune canari


etc...

Feel free to comment, any help is appreciated !

• The singular of 'species' is 'species'; 'specie' means coined money. – David Conrad Jun 30 '16 at 18:55
• By using the non ascii characters you should be using. Especially the colors names should be downcased and many accents are missing eg: Heliotrope => héliotrope (not a color by the way). – Antzi Jul 1 '16 at 5:37
• thanks for the comment, I didn't knew about specie/species. I'm actually french. I know about the accent, and I removed them because they were counted as 2 caracters in the strings, but now I know how to do it better. There are relatively strange color names, they all come from the global wiki page about colors, but I like that. – dogspeed Jul 1 '16 at 7:10

Use classes

You should consider making animals a class, since you intend to make many of them that all have the same types of characteristics. This is exactly what classes are good for. I've taken the liberty of creating a characteristics dictionary for easy characteristic assignment using the data read from files

Don't repeat yourself

You do most things 5 times in a row (one for each characteristic). You can avoid this in most situations. For reading data, you can stuff each set of characteristics into one element of a dictionary. For assigning characteristics, you can randomly select one item from each element of that dictionary.

Here's a crack at your code, revised:

import random

documents = [
"ani_nom.txt",
"ani_ori.txt",
"ani_cor.txt",
"ani_cou.txt"
]

data = {}

for document in documents:
try:
data.update({document.split(".")[0]: [line.strip() for line in open(document)]})
finally:
close(document)

class Animal(object):
def __init__(self):
self.characteristics = dict(
ani_nom=None,
ani_ori=None,
ani_cor=None,
ani_cou=None
)

def assign_all_characteristics(self):
for characteristic in self.characteristics:
self.characteristics[characteristic] = random.choice(data[characteristic])

def print_random_characteristics(self):
num_to_print = random.randint(1, 5)
characteristics_to_print = list(self.characteristics.values())
print "\n".join(characteristics_to_print[:num_to_print])

number_of_animals = 120
for _ in range(number_of_animals):
animal = Animal()
animal.assign_all_characteristics()
animal.print_random_characteristics()

• Thanks ! I never used classes before that, so that's a good starting point. I think I kind of get the characterics assignment part, but can you tell me a bit more about the lines 13-14, and also the last one ? How does this work ? – dogspeed Jun 30 '16 at 15:57
• Lines 13 and 14 create a dictionary. The keys of the dictionary are document.split(".")[0], which is going to be ani_nom, ani_adj, etc. The data is going to be [line.strip() for line in open(document).readlines()] Although, as others have pointed out, readlines is not necessary and this does not close the file. – Jeremy Weirich Jun 30 '16 at 18:07
• I'm guessing by last line, you mean the print_random_characteristics method. Basically, all of the characteristics of the animal are grouped into a list. I get a slice of the list with characteristics_to_print[:num_to_print] and then I print it. However, you print your characteristics one on each line. To accommodate that, I join each string with the newline character \n, – Jeremy Weirich Jun 30 '16 at 18:09
• What is the advantage of dict.uptate with a dict literal with a single key? Why isn't the indexing assignment sufficient? – Kroltan Jul 1 '16 at 1:26
• Personal preference. Subscript notation is sufficient - here is a discussion of all options. – Jeremy Weirich Jul 1 '16 at 12:29

Here are some suggestions:

Use descriptive variable names

use more descriptive names, such as word_length, instead of z, etc.

Use functions to avoid repetition

As it is, there is quite a bit of repetition on your code. The first is the reading in of the file. I suggest to use something like this:

def read_file(file_name):
return [line.strip() for line in open(file_name).readlines()]


Similarly, you should put the printing part of the code into a function as well:

def print_animal(words, mode):
assert(mode in range(1, 5))
s = words["name"]
if mode in (3, 5):
s += " " + words["adjective"]
if mode in (1, 4, 5):
s += " " + words["origin"]
if mode > 1:
s += " " + words["bodypart"] + " a " + words["color"]
print s


This builds a string first, using some simplifications of your logic, and then prints it. Note, if you don't change the filenames, you will have to change here the keys of the words dictionary as well. This string has the words separated by spaces instead of newlines. I think this makes it more natural to read. If you do need/want the newlines, you will have to replace the appropriate spaces with newlines (\n).

Note that your branch of r == 0 will never be called, because random.randint(1,5) generates a random integer in the closed range [1,5].

Use list/dictionary comprehensions

Become familiar with comprehensions, they allow you to avoid a lot of code duplication. They allow you to do for example:

WORD_TYPES = ["name", "adjective", "origin", "bodypart", "color"]
ALL_WORDS = {word_type: read_file("ani_"+word_type+".txt") for word_type in word_types}


The above comprehension loops over the different input files and stores the resulting words as lists in a dictionary where the key is the word type (name, origin, ...).

For this code to work, you'd have to rename the files to the format ani_name.txt, ani_adjective.txt and so forth. Since I don't speak French very well, it was easier for me to use more distinctive names here.

Btw, global variables should be written in CAPITAL_LETTERS to distinguish them from local variables.

Similarly, you can use them to define first the words of the right length:

word_length = random.randint(1, 100)
words = {word_type: filter(lambda word: len(word) == (word_length, word_length-2)[word_type == "bodypart"], ALL_WORDS[word_type]) for word_type in WORD_TYPES}


Here the (word_length, word_length-2)[word_type == "bodypart"] part is a short way to give a word of length word_length-2 if a body-part is requested and of length word_length otherwise, since python uses True == 1 and False == 0

For the words chosen to make up the description you can also use a dictionary comprehension:

chosen words = {word_type: random.choice(words[word_type]) for word_type in WORD_TYPES}


You will still need the try...except IndexError part in case one of the word types does not contain any word with the requested length. Since the word_length can be up to 100 this will likely happen quite often, so you might end up with a lot less than 120 descriptions. Maybe calculate the minimum maximum word_length (above which you will always hit the IndexError for one of the word_types), to ameliorate this somewhat:

MIN_MAX_LENGTH = min([max([len(word) for word in ALL_WORDS[word_type]]) for word_type in WORD_TYPES])


See below.

Result

My final code using the above suggestions (untested):

#!/usr/bin/env python
import time
import random

# Program to read in word parts from the files
# 'ani_name.txt', 'ani_adjective.txt', 'ani_origin.txt', 'ani_bodypart.txt', 'ani_color.txt' and construct 120 random descriptions of animals from them. The words in each description of an animal are ensured to have the same length.

"""Return a list of all whitespace-stripped lines taken from file_name"""
return [line.strip() for line in open(file_name).readlines()]

def print_animal(words, mode):
"""Print a description of an animal, given a dictionary of words with different word_types as key and a mode detailing which word_types to use
mode
1) prints name and origin
2) prints name and a bodypart with an associated color
3) prints name, adjective and a bodypart with an associated color
4) prints name, origin and a bodypart with an associated color
5) prints name, adjective, origin and a bodypart with an associated color
"""
s = words["name"]
if mode in (3, 5):
s += " " + words["adjective"]
if mode in (1, 4, 5):
s += " " + words["origin"]
if mode > 1:
s += " " + words["bodypart"] + " a " + words["color"]
print s

WORD_TYPES = ["name", "adjective", "origin", "bodypart", "color"]
ALL_WORDS = {word_type: read_file("ani_"+word_type+".txt") for word_type in WORD_TYPES}
MAX_MIN_LENGTH = max([min([len(word) for word in ALL_WORDS[word_type]]) for word_type in WORD_TYPES])
MIN_MAX_LENGTH = min([max([len(word) for word in ALL_WORDS[word_type]]) for word_type in WORD_TYPES])

for n in range(120):
word_length = random.randint(MAX_MIN_LENGTH, MIN_MAX_LENGTH)
words = {word_type: filter(lambda word: len(word) == (word_length, word_length-2)[word_type == "bodypart"], ALL_WORDS[word_type]) for word_type in WORD_TYPES}
try:
chosen words = {word_type: random.choice(words[word_type]) for word_type in WORD_TYPES}
except IndexError:
continue
print_animal(chosen_words, mode=random.randint(1, 5))


Further improvements

Use command line arguments. Support at least giving n (the number of descriptions generated), maybe even the filenames of the word lists.

Maybe combine the word lists and add a column specifying what kind of word type each word is. This will lead to a lot of modifications, though.

Don't use a magic integer as mode for print_animal, but build a list of included word_types, so its signature becomes simply print_animal(word, word_types)

Even better, build an animal object and only fill in the needed parts to describe it. Give it a print function that prints all existing parts.

• Should have refreshed while writing my answer... – Graipher Jun 30 '16 at 16:42

I'd change how you get Inom, Iadj, etc. Instead of making them each iteration you can make them all once, and be done with it. I would also put them in a dictionary, and make a dictionary for nom, adj, etc.

The way that I'd store Inom is so that you have a dictionary in it that has a list of every size z in it. Apart form when it's cor, where you minus the size by two. And so I'd have:

words = {
'nom': [line.strip() for line in open("ani_nom.txt").readlines()],
'ori': [line.strip() for line in open("ani_ori.txt").readlines()],
'cor': [line.strip() for line in open("ani_cor.txt").readlines()],
'cou': [line.strip() for line in open("ani_cou.txt").readlines()],
}

words = {
key: {
size: [i for i in value if len(i) == (size - 2 if key == 'cor' else size)]
for size in xrange(1, 101)
}
for key, value in words.iteritems()
}


This allows you to change you while loop to:

for w in range(120):
z = random.randint(1, 100)
i_word = {
key: value[z]
for key, value in words.iteritems()
}
try:
name = random.choice(i_word['nom'])


From this I'd change your try to an if. If any of list in i_word are empty you'll error and will enter the except. And so you can change this to use all and i_word.itervalues().

if not all(i_word.itervalues()):
continue


You can then apply the random on this dictionary knowing it won't error.

val = {
key: random.choice(value)
for key, value in i_word.iteritems()
}


After this is where a dictionary becomes very helpful. Using str.format you can change your prints to:

if r == 0:
print '{nom}'.format(**val)
if r == 1:
print '{nom}\n{ori}'.format(**val)


And then you can put all the formats in a list, and apply the format on it.
This all results in:

import random
words = {
'name': [line.strip() for line in open("ani_nom.txt").readlines()],
'orig': [line.strip() for line in open("ani_ori.txt").readlines()],
'corp': [line.strip() for line in open("ani_cor.txt").readlines()],
'coul': [line.strip() for line in open("ani_cou.txt").readlines()],
}

formats = [
'{name}',
'{name}\n{orig}',
'{name}\na {corp}\n{coul}',
'{name}\n{orig}\na {corp}\n{coul}',
]

words = {
key: {
size: [i for i in value if len(i) == (size - 2 if key == 'cor' else size)]
for size in xrange(1, 101)
}
for key, value in words.iteritems()
}

for w in range(120):
z = random.randint(1, 100)
i_word = {
key: value[z]
for key, value in words.iteritems()
}
if not all(i_word.itervalues()):
continue
val = {
key: random.choice(value)
for key, value in i_word.iteritems()
}
r = random.randrange(1,5)
print ""
print formats[r].format(**val)


I more or less rewrote this to use classes (but keeping the same generation logic you are using - you may wish to change it but I'm not sure how to change it with your minimal information).

Right now, it is difficult to add or remove different elements and you have a lot of repetition.

First you have some extra stuff at the top of your class, all you need is:

import random


Animals are all basically the same here, they have different attributes and you want to print them. In the future you may end up with your attributes actually being member variables, but right now just a list of them is sufficient:

class Animal:

def __init__(self):
self.attributes = ['']

def print(self):
for a in self.attributes:
print a


You also want a way to better generate these. I've created an AnimalFarm class:

class AnimalFarm:

def __init__(self):
self._config = {}
self._r = 5

self._config[name]['data'] = [line.strip() for line in open(file).readlines()]
self._config[name]['print_vals'] = print_vals
self._config[name]['print_fmt'] = print_fmt

def generate_animals(self, numb):
animals = []
for w in range(numb):
animals.append(self._generate_animal())

return animals

def _generate_animal(self):
r = random.randint(1, 5)
z = random.randint(1,100)
animal = Animal()
for key in self._config:
d = self._config[key]
matches = [x for x in d['data'] if len(x) == z + d['len_adjust']]
if len(matches) > 0 and r in d['print_vals']:
val = random.choice(d)
animal.attributes.append(d['print_fmt'].format(val))

return animal


This is more complicated, because it uses provided configuration information for whatever attribute you need and then auto generates the right number of animals.

You can use it like:

farm = AnimalFarm()
farm.add_config('cor', 'ani_cor.txt', range(2,6), 'a {}', -2)


And then once you've configured it:

for animal in farm.generate_animals(120):
animal.print()


General observations:

• If you find yourself copying/pasting something 5 times it probably should be a function
• Any time you have a conceptual "grouping of data" it probably should be a class, such as animals
• Use v = 5 for spacing instead of v=5 as it reads better and is more pythonic

1. You don't need readlines. You can just iterate over files.
2. You are leaving your files open. You should open them with with, this will close them automatically when you are doing with them.
3. You initialize z outside the loop, but then immediately create a new z at the beginning of the loop, so it is never used.
4. You pick a random integer, then find if any values match it. This requires you to do one search for each part, even if there is nothing that matches. I think it would be better to pick one value from one list, then find all the values from the other list that match that length.
5. Better yet, I think it would be better to pre-compute the values based on length, then pick a random length, then pick a random value based on that length.
6. Your elifs are somewhat redundant, you can simplify them somewhat.

So here is how I would do it:

import random

# Pre-define the dict of groups
# Groups are defined based on string length
groups = {}

# The files to open
fnames = ["ani_nom.txt", "ani_adj.txt", "ani_ori.txt", "ani_cor.txt", "ani_cou.txt"]

# Loop over the file names
for fname in fnames:
# Open the file safely
with open(fname, "r") as fobj:
# Loop over lines, stripping as we go
lines = (line.strip() for line in fobj)

# If we are working with cor, append the "a " now to make it easier later
if fname == "ani_cor.txt":
lines = ("a "+line for line in lines)

# Determine the lengths of the strings
lens = ((len(line), line) for line in lines)

# Create a dict where the key is the length and the value is a list of
# all lines with that length
lendict = {}
for ilen, line in lens:
lendict.setdefault(ilen, []).append(line)

# If this is the first file, use it to create the final dict
if fname == "ani_nom.txt":
groups = {key: [value] for key, value in lendict.iteritems()}
continue

# If this is the second file, append its values to the corresponding values
# of the main dict, dropping any non-existing lengths from the main dict
for key, value in groups.items():
if key in lendict:
value.append(lendict[key])
else:
del groups[key]

# Convert the values to a list, since we don't actually care about the
# line length value, we just use it for grouping
groups = groups.values()

for w in range (120):
# Randomly select a group
group = random.choice(groups)

# Randomly select one value from each item in the group and put it in
# the correct variables
name, adje, orig, corp, coul = (random.choice(parts) for parts in group)

r = random.randint(1,5)
print ""
# We always do this
print name

# Simplify the if tests
if r in {3, 5}:
if r in {1, 4}:
print orig
if r > 1:
print corp
print coul


Linguistic

Working with French is more challenging, because adjectives must agree with nouns. Here, you haven't made any attempt to pick between "blanc" and "blanche", for example. (Frankly, that is a tricky problem to solve correctly.)

This combination looks wrong, because the 'ç' is being counted as two bytes:

bronze
maçon
de mer
a aile
Soufre


Assuming that you are storing the text as UTF-8 (which is preferred over ISO-8859-1 and other legacy encodings), you need to decode the files as UTF-8. I suggest using codecs.open(filename, 'r', 'utf-8').

Also, you should put -*- coding: utf-8 -*- at the top of the program so that you can write u"à " + corp.

Style

I can understand nom and adj, but not ori, cor, or cou. The satisfaction of having words aligned in your code is not worth the decrease in readability due to your meaningless abbreviations!

This line, for example, is problematic:

Icor=[x for x in cor if len (x) == z-2 ]

• z is a poor variable name. width would be better.
• Icor is a poor variable name. corps_with_width would be better.
• z-2 is a weird special case. You should have prepended u"à " immediately upon reading the file instead.

I strongly suggest that you define some functions. Here, you could easily define a function for reading a file, since that is duplicated code. You should also define a function for generating an animal, since that is the primary functionality.

Algorithm

100 is a magic number. Set it too low, and you might exclude some valid choices. Set it too high (which you probably have done), and you could have massive inefficiency as you keep retrying to select a usable width.

It would be much better to know what the allowable widths are. You can determine the allowable widths when reading each file. Better yet, segregate each line by width while you read each file.

5 is also a magic number: it corresponds to the five possible templates. I suggest making a list of all the possible templates and calling random.choice() to select one at random.

Your algorithm is incapable of generating some possibly valid results. For example, if there is a "Heterodontosaurus" in nom but no 12-letter place of origin, then "Heterodontosaurus" will never appear in the results, even if 0 is chosen for r.

Suggested solution

# -*- coding: utf-8 -*-

import codecs
from collections import defaultdict
import random

def animal_component(filename, transform=lambda s:s):
"""
Read lines from a file, optionally transforming each line.
Return a dictionary whose keys are the line lengths and whose
values are a list of lines with that length.
"""
lines = defaultdict(list)
with codecs.open(filename, 'r', 'utf-8') as f:
for line in f:
line = transform(line.strip())
lines[len(line)].append(line)
return lines

def random_animal(templates):
"""
Pick a random animal combination, given a list of lists of animal components.
"""
while True:
template = random.choice(templates)
component_widths = [set(component.keys()) for component in template]
allowable_widths = set.intersection(*component_widths)
if not allowable_widths:
continue
chosen_width = random.choice(list(allowable_widths))
return [random.choice(component[chosen_width]) for component in template]

noms = animal_component('animal_noms.txt')
origines = animal_component('animal_origines.txt')
corps = animal_component('animal_corps.txt', lambda corps: u'à ' + corps)
couleurs = animal_component('animal_couleurs.txt')

templates = [
[noms],
[noms, origines],
[noms, corps, couleurs],
[noms, origines, corps, couleurs],
]

for _ in range(120):
animal_words = random_animal(templates)
print('\n'.join(animal_words))
print


This is a pretty basic answer, a lot has already been covered. This block is bugging me a bit though:

for w in range (120):
z=random.randint(1,100)
Inom=[x for x in nom if len (x) == z ]
Iori=[x for x in ori if len (x) == z ]
Icor=[x for x in cor if len (x) == z-2 ]
Icou=[x for x in cou if len (x) == z ]
try:
name = random.choice(Inom)
orig = random.choice(Iori)
corp = random.choice(Icor)
coul = random.choice(Icou)


Most of your variable and file names are 1. French and 2. abbreviated. They are also not consistent. The Ithing naming is not really that great either. If you can, at least use names that actually describe the things they represent, and use the same language for all of them, e.g.

for w in range (120):
z = random.randint(1,100)
Name = [x for x in names if len (x) == z ]
Adjective = [x for x in adjectives if len (x) == z ]
Origin = [x for x in origins if len (x) == z ]
BodyPart = [x for x in body_parts if len (x) == z-2 ]
Color =[x for x in colors if len (x) == z ]
try:
name = random.choice(Name)
origin = random.choice(Origin )
body_part = random.choice(BodyPart)
color = random.choice(Color)


If you still want them to look aligned vertically in the code, just use spaces/tabs, although that's often not necessarily desired...

for w in range(120):
z = random.randint(1,100)
Name      = [x for x in names      if len (x) == z ]
Adjective = [x for x in adjectives if len (x) == z ]
Origin    = [x for x in origins    if len (x) == z ]
BodyPart  = [x for x in body_parts if len (x) == z-2 ]
Color     = [x for x in colors     if len (x) == z ]
try:
name      = random.choice(Name)