replacing words with their abbreviations

Question

I'm working on a program that aim to take sentences (currently in french) and compact them to a length of 38 characters while retaining as much information as possible.
You can find another part of this project here Remove determiners in a string
You can also find a follow up for this question here: Replacing words with their abbreviations - Follow up

This particular function's goal is to replace words with their abbreviations by compairing each word with all of those in the config file and replacing whenever a match is found, the goal is to keep a géneral idea of what the string meant so its not a problem that a word once replaced could ressemble an other as long as they refer to roughly the same thing.
String is always upper case and free of special characters.

Here is how I do it:

def shorten_words(abbreviations, string):
    # abbreviations is a file parsed with configparser
    for key in abbreviations:

        # finds beginning of possible words
        it_string = 0
        while it_string < len(string) and len(string) > 38:
            if string[it_string] == ' ' or it_string == 0:
                if it_string == 0:
                    it_string = -1

                # finds end of possible words
                it_word = 0
                while(it_word < len(key) and it_string + it_word + 1 < len(string)
                     and string[it_string + it_word + 1] == key[it_word].upper()):
                    it_word += 1

                # cuts the line
                if(it_word == len(key) and (it_string + it_word + 1 == len(string)
                     or string[it_string + it_word + 1] == ' ')):
                    string = string[:it_string + 1] + abbreviations[key].upper() + string[it_string + it_word + 1:]

                # cuts the line for the same word with an 'S' at the end
                elif(it_word == len(key) and (string[it_string + it_word + 1] == 'S'
                     and (it_string + it_word + 2 == len(string)
                     or string[it_string + it_word + 2] == ' '))):
                    it_word += 1
                    string = string[:it_string + 1] + abbreviations[key].upper() + string[it_string + it_word + 1:]

            it_string += 1
            if(it_string == 0):
                it_string = 1
    return(string)

here is a sample of the configfile (if key and value are equal the line is made to remove plural):

[abbreviation]
AVANCEE = AVANC
COMPOSANT = COMPO
VERT = VERT
AGRAIRE = AGRAIR
MECANIQUE = MECA
CARROSSERIE = CARROS
SIGNALISATION = SIGNAL
FOURNITURE = FOURNI
LAITIERE = LAIT
INTERPROFESSIONNEL = INTRPRO
ATLANTIQUE = ATLAN
REALISATION = REAL
INCENDIE = INCEND
MARBRERIE = MARB
FUNEBRE = FUNEBR
POMPE = POMPE
ANTICIPATION = ANTICIP
OBJET = OBJET
ANTIQUITE = ANTIQ
MOBILITE = MOBIL
ASSOCIATIF = ASSO
ANCIENNE = ANC
TELECOMMUNICATION = TELECOM
RESEAUX = RESEAU
LOCALE = LOCAL
RESPIRE = RESPI
QUAND = QND
CHRETIENNE = CHRET
OUVRIERE = OUVRI
JEUNESSE = JEUNE
INTERCULTUREL = INTRCULT
VALORISATION = VALOR
ALIMENTAIRE = ALIMEN
COMMUNALE = COMMUNE
LAIQUE = LAIQ
CASSATION = CASS
TRAVAUX = TRAVAU
ONCOLOGIE = ONCO
RELIGION = RELIG
PLURALISME = PLURAL
FLOTTANTE = FLOT
EOLIENNE = EOLIEN
HUMAINE = HUMAIN
POTENTIEL = POTENT
AMELIORATION = AMELIO
MUSIQUE = MUSIQ
MUNICIPALE = MUNI
EVANGELIQUE = EVANG
BIOLOGISTE = BIOLOG
REPUBLICAIN = REPU
SYMPATHISANT = SYMPAT
ELU = ELU
INTERCONNEXION = INTRCONN
CONSULTANT = CONSULT
ORGANIZATION = ORGA
OLYMPIQUE = OLYMP
CAPACITE = CAPA
RENFORCEMENT = RENFOR
CLEF = CLEF
FRIGORIFIQUE = FRIGO
ENTREPOSAGE = ENTREPO
COLLABORATIF = COLLAB
TROUBLE = TROUBL
ENTRAIDE = ENTRAID
REPRESENTANT = REPRESENT
ADHERENT = ADHER
FOLKLORIQUE = FOLKLO
STADE = STAD
AMI = AMI
EMPEREURS = EMPER
CONFRERIE = CONFRER
SOUTENUE = SOUTENU
LISTE = LIST
ELECTION = ELECT
ELECTORALE = ELECT
FINANCEMENT = FINANC
CATHOLIQUE = CATHO
HARMONIE = HARMO
DEBOUT = DEBOU
VENT = VENT
CERCLE = CERCL
FOOTBALL = FOOT
IMPROVISATION = IMPROV
POPULAIRE = POPU
SECOURS = SECOUR
ART = ART
DRAMATURGIE = DRAMA
POETIQUE = POET
TRAVAILLANT = TRAVAIL
SYNCHRONISEE = SYNCHRO
NATATION = NATA
LOCATAIRES = LOCAT
AMICALE = AMICA
DEPARTEMENT = DEPART
INDISCIPLINEE = INDISCIPL
PARTAGE = PARTA
MEDIATION = MEDIAT
CITOYEN = CITOY
CULTIVONS = CULTIV
QUARTIER = QUART
DOMICILE = DOMI
ADMINIS = ADMIN
APPLIQUEE = APPLI
SOPHROLOGIE = SOPHRO
SPECTACLE = SPECTA
ABANDONNE = ABANDON
COMMUNAUTAIRE = COMMUN
PARTICULIER = PARTICUL
METALLIQUE = METAL
COOPERATION = COOP
PROGRAMMATION = PROGRAM
KINESITHERAPEUTE = KINESITHERAP
ENVIRON = ENVIRON
ARTISAN = ARTIS
COMMUNICATION = COM
TRANSMISSION = TRANSMIS
APPROVISIONNEMENT = APPRO
IMAGERIE = IMAGE
MANAGEMENT = MANAG
ASSOCIEE = ASSO
INFIRMIERE = INFIRM
FONDS = FOND
EMBOUTISSAGE = EMBOUTISS
DECOUPAGE = DECOUP
OUTILLAGE = OUTIL
TERRASSEMENT = TERRASS
DEMOLITION = DEMOLIT
BILINGUE = BILINGU
ECOLE = ECOL
HABITAT = HABITA
PRODUCTION = PROD
DURABLE = DURABL
PRATIQUE = PRATIQ
TRANSPORT = TRANSPOR
ASSOCIATIVE = ASSO
CRECHE = CRECH
SPECIALISEE = SPECIAL
COUVERTURE = COUVERT
ETANCHEITE = ETANCH
TOITURE = TOIT

Answer 1

reinvent the wheel.

what is wrong with str.replace

unclear variable names

It was unclear to me what it_string and it_word are until I've looked at all the code. pick variable names that express what the purpose of the variable is

magic numbers

you use 38 as a magic number. It is unclear where this comes from. This is the maximum length a line should be, then extract a variable in your function to say so. You can also make this an argument to the function with default value 38.

line length

Try to limit the line length. For long expression (the if and while classes, I would split the across lines. To help me be consistent here, I use black

Shadow standard modules

string is a standard module, so I would pick another name for this argument.

Alternative approach

In python, it is very easy to split a line into words (line.split(' ')), So you can work per word instead of per character, and then look that word up in the abbreviations dict

def shorten_words(abbreviations, line, max_length=38):
    while len(line) > max_length:
        for word in line.split("\t"):
            if word in abbreviations or word + "S" in abbreviations:
                line = line.replace(word, abbreviations[word])
                break
    return line

This will enter an endless loop if word == abbreviations[word]. To counter that, you need to keep a set of replaced words. I also made a mistake in the handling of multiples:

def shorten_words(abbreviations, line, max_length=38):
    replacements = set()
    while len(line) > max_length:
        for word in line.split(" "):
            if (
                word[-1] == "S"
                and word not in abbreviations
                and word[:-1] in abbreviations
            ):
                word = word[:-1]
            if word not in replacements and word in abbreviations:
                line = line.replace(word, abbreviations[word])
                if word == abbreviations[word]:
                    replacements.add(word)
                break
    return line

testing

My mistakes show the importance of testing code. Make some unittests to see if the code does what you want it to do, and run them each time you change the code. A good IDE can be a large help here.

Answer 2

Comments from answers on your other questions

From Julien Rousé's answer:

Documentation

You have comments, that's good. But it is way better to have a good docstring for the function, and fewer comments.

Your docstring should describe what the function try to accomplish, why it exists, the parameter (eventually their type) and what does the function return.

Also, there are Python Docstrings conventions (which are not always fully followed but they are definitly worh reading).

Indentation

Your code here: is very hard to read. Maybe it's because you pasted it into CodeReview, but be carefull when writing long and hard condition. The more difficult it is to read, the more difficult it is to debug/maintain/extend.

From l0b0's answer:

Naming is really important. string, array, i and j are not descriptive. After reading the entire function I think they could be renamed sentence, words, string_index and word_length.

Then

What is the significance of 38? If it's not significant it should be removed, if it is it should be named something like max_result_length.

And

In Python return is a simple statement, which means its argument should not be put in parentheses.

---

My own comments

Style

Python has a Style Guide called PEP 8. It is worth reading and trying to apply. Among other things it gives advices about naming, spacing, etc.

Tests

Your function takes 2 inputs and return an output. It would be a nice touch to write a few tests for it. (It may be worth pointing out that the abbreviations parameter does not need to be a config file object, any object acting like a dictionnary would do the trick. This can make writing tests easier and more explicit and you do not need to use external modules to load config files nor to have configuration in a different file.

Taking the most out of dicts

You can iterate over pairs of (key, values) on a dictionnary using items(). Also, you can take this chance to rename (key, val) to (longword, shortword) to be more explicit.

Work in progress: at this stage, I have

MAX_RESULT_LENGTH = 38

def shorten_words(abbreviations, string):
    """Shorten string `str` using the dictionnary_like object `abbreviations`."""
    for longword, shortword in abbreviations.items():

        # finds beginning of possible words
        it_string = 0
        while it_string < len(string) and len(string) > MAX_RESULT_LENGTH:
            if string[it_string] == ' ' or it_string == 0:
                if it_string == 0:
                    it_string = -1

                # finds end of possible words
                it_word = 0
                while (it_word < len(longword) and
                       it_string + it_word + 1 < len(string) and
                       string[it_string + it_word + 1] == longword[it_word].upper()):
                    it_word += 1

                # cuts the line
                if (it_word == len(longword) and
                    (it_string + it_word + 1 == len(string) or string[it_string + it_word + 1] == ' ')):
                    string = string[:it_string + 1] + shortword.upper() + string[it_string + it_word + 1:]
                # cuts the line for the same word with an 'S' at the end
                elif (it_word == len(longword) and
                      (string[it_string + it_word + 1] == 'S' and
                         (it_string + it_word + 2 == len(string) or string[it_string + it_word + 2] == ' '))):
                    it_word += 1
                    string = string[:it_string + 1] + shortword.upper() + string[it_string + it_word + 1:]

            it_string += 1
            if it_string == 0:
                it_string = 1
    return string