Thanks for sharing your code.
It's a nice project you have there.
Naming
You should take some time to choose carefully your variable. l1
is not obvious, maybe input
, input_string
, sentence
or text
would be more appropriate?
Without the commentary above the function, it would have been hard to find what that was supposed to be.
Same for it_l1
and it_word
. After reading your code a few time, I am not sure what they are. Find a descriptive name, it helps tremendously when reading code.
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.
And more annoying, some of your comments seem to be outdated/wrong?
# determiners is the config file below parsed by configparser
but it seems determiners
is not a config file, but a list of string. (True it is the content of a file parsed by configparser
, but it's not the same)
# the goal is to obtain a string shorter than 38 char
you did change your code but did not change that comment I presume? I couldn't find where it was supposed to restrict the size to 38char
Indentation
Your code here:
while(it_word < len(key) and it_l1 + it_word + 1 < len(l1)
and l1[it_l1 + it_word + 1] == key[it_word].upper()):
it_word += 1
if(it_word == len(key) and (it_l1 + it_word + 1 == len(l1)
or l1[it_l1 + it_word + 1] in ' -.')):
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.
Putting it altogether
Note: it_l1
and it_word
are still in the code because I don't know how to name them properly but you should probably change them.
def remove_determiners(determiners, text):
"""
Removes determiners words from text
:param determiners: the words to remove in `text`
:param text: a text to remove determiner in
:type determiners: List[str]
:type text: str:
:return: The value of `text` after removing all words present in determiners
"""
for key in determiners:
it_l1 = 0
while(it_l1 < len(text)):
# Finds possible beginning of words
if(text[it_l1] in ' -' or it_l1 == 0):
if(it_l1 == 0):
it_l1 = -1
# Finds possible end of words
it_word = 0
while(it_word < len(key)
and it_l1 + it_word + 1 < len(l1)
and text[it_l1 + it_word + 1] == key[it_word].upper()):
it_word += 1
if(it_word == len(key)
and (it_l1 + it_word + 1 == len(text)
or text[it_l1 + it_word + 1] in ' -.')):
if(it_l1 == -1):
it_l1 = 0
# cuts the determiner out
text = l1[:it_l1] + l1[it_l1 + it_word + 1:]
it_l1 += 1
if(it_l1 == 0):
it_l1 = 1
return(text)
Algorithm
You could also really simplify your code by using set.
Below is a code that works for the simple case, need to be improved to deal with punctuation. (if you have a text like "LE, LA, ET.", it will not remove the determiners. Or use a clean up step as shown below)
def remove_determiners(determiners, text):
"""
Removes determiners words from text
:param determiners: the words to remove in `text`
:param text: a text to remove determiner in
:type determiners: List[str]
:type text: str:
:return: The value of `text` after removing all words present in determiners
"""
determiners_set = set(determiners)
text_list = text.split(' ')
resultat = []
for element in text_list:
if(element not in determiners_set):
resultat.append(element)
return ' '.join(resultat)
Or as a two-liner using list comprehension as @Josay pointed out in the comment:
def remove_determiners(determiners, text):
determiners_set = set(determiners)
return ' '.join(e for e in text.split(' ') if e not in determiners_set)
Usage:
determiners = ['AND',
'THE',
'ET',
'LE',
'LA',
'LES',
'DE']
text = 'LES PILES DE LA TELECOMMANDE'
remove_determiners(determiners, text)
# returns 'PILES TELECOMMANDE'
Sidenote
Why in you config file you have & = nope
instead of &
? Is there more values than nope
?
EDIT
Code changed slightly since my answer, adding the 38 char constraint. Should be straightforward to modify my response to accomodate this change
EDIT2
Also to address @Josay point about noise in the text, a cleaning step is possible like this:
def clean_text(text)
"""Remove ponctuation from text and replace it by a empty char
for -_,.?!:; and '" by a space
:param text: The text to remove punctuation in
:return: Text cleaned up
"""
text = re.sub('[-_,.?!:;]', '', text)
text = re.sub('[\'"]', ' ', text)
return text
And usage
determiners = ['AND',
'THE',
'ET',
'LE',
'LA',
'LES',
'DE']
text = 'LES PILES DE-LA TELECOMMANDE'
remove_determiners(determiners, clean_text(text))
remove_determiners
? \$\endgroup\$