Here are a few suggestions on how to improve the code:
State the intent###intent
What is the purpose of the function? What is the input, what is the output? In human words, what does the algorithm do? A docstring answering those basic questions would be helpful. A series of concise unit tests would be great.
I ran the function to have a look at the output:
>>> text = "The weather is not good."
>>> result = negate_sequence(self=None, text=text)
>>> print(result)
['the', 'weather', 'the weather', 'is', 'weather is', 'the weather is', 'not', 'is not',
'weather is not', 'not_good', 'not not_good', 'is not not_good']
This doesn't ring a bell with me, so I stopped trying to understand the purpose.
Avoid stateful loops
Iteration i is coupled to iteration i-1 by the negation variable, this makes the logic hard to understand and error prone. If you work on bigrams/trigrams, I'd create a list of bigrams/trigrams and iterate over the tuples. This decouples the iterations.
Breakup long functions
This has almost endless benefits, as a starting point see this article. Some possibilities:
- Have the text broken up into all lowercase and without punctuation by
extract_words(text) - Have the list of trigrams created by
make_trigrams(words) - Inspect the trigrams by
process(trigrams) - If needed, have some kind of
aggregate(results)
Once this is done, I guess we are much better to prepared to identify bugs and to further improve functionality.
Here are a few suggestions on how to improve the code:
State the intent###
What is the purpose of the function? What is the input, what is the output? In human words, what does the algorithm do? A docstring answering those basic questions would be helpful. A series of concise unit tests would be great.
I ran the function to have a look at the output:
>>> text = "The weather is not good."
>>> result = negate_sequence(self=None, text=text)
>>> print(result)
['the', 'weather', 'the weather', 'is', 'weather is', 'the weather is', 'not', 'is not',
'weather is not', 'not_good', 'not not_good', 'is not not_good']
This doesn't ring a bell with me, so I stopped trying to understand the purpose.
Avoid stateful loops
Iteration i is coupled to iteration i-1 by the negation variable, this makes the logic hard to understand and error prone. If you work on bigrams/trigrams, I'd create a list of bigrams/trigrams and iterate over the tuples. This decouples the iterations.
Breakup long functions
This has almost endless benefits, as a starting point see this article. Some possibilities:
- Have the text broken up into all lowercase and without punctuation by
extract_words(text) - Have the list of trigrams created by
make_trigrams(words) - Inspect the trigrams by
process(trigrams) - If needed, have some kind of
aggregate(results)
Once this is done, I guess we are much better to prepared to identify bugs and to further improve functionality.
Here are a few suggestions on how to improve the code:
State the intent
What is the purpose of the function? What is the input, what is the output? In human words, what does the algorithm do? A docstring answering those basic questions would be helpful. A series of concise unit tests would be great.
I ran the function to have a look at the output:
>>> text = "The weather is not good."
>>> result = negate_sequence(self=None, text=text)
>>> print(result)
['the', 'weather', 'the weather', 'is', 'weather is', 'the weather is', 'not', 'is not',
'weather is not', 'not_good', 'not not_good', 'is not not_good']
This doesn't ring a bell with me, so I stopped trying to understand the purpose.
Avoid stateful loops
Iteration i is coupled to iteration i-1 by the negation variable, this makes the logic hard to understand and error prone. If you work on bigrams/trigrams, I'd create a list of bigrams/trigrams and iterate over the tuples. This decouples the iterations.
Breakup long functions
This has almost endless benefits, as a starting point see this article. Some possibilities:
- Have the text broken up into all lowercase and without punctuation by
extract_words(text) - Have the list of trigrams created by
make_trigrams(words) - Inspect the trigrams by
process(trigrams) - If needed, have some kind of
aggregate(results)
Once this is done, I guess we are much better to prepared to identify bugs and to further improve functionality.
Here are a few suggestions on how to improve the code:
State the intent###
What is the purpose of the function? What is the input, what is the output? In human words, what does the algorithm do? A docstring answering those basic questions would be helpful. A series of concise unit tests would be great.
I runran the function to have a look at the output:
>>> text = "The weather is not good."
>>> result = negate_sequence(self=None, text=text)
>>> print(result)
['the', 'weather', 'the weather', 'is', 'weather is', 'the weather is', 'not', 'is not',
'weather is not', 'not_good', 'not not_good', 'is not not_good']
This doesn't ring a bell with me, so I stopped trying to understand the purpose.
Avoid stateful loops
Iteration i is coupled to iteration i-1 by the negation variable, this makes the logic hard to understand and error prone. If you work on bigrams/trigrams, I'd create a list of bigrams/trigrams and iterate over the tuples. This decouples the iterations.
Breakup long functions
This has almost endless benefits, as a starting point see this article. Some possibilities:
- Have the text broken up into alllowercaseall lowercase and without punctuation by
extract_words(text) - Have the list of trigrams created by
make_trigrams(words) - Inspect the trigrams by
process(trigrams) - If needed, have some kind of
aggregate(results)
Once this is done, I guess we are much better to prepared to identify bugs and to further improve functionality.
Here are a few suggestions on how to improve the code:
State the intent###
What is the purpose of the function? What is the input, what the output? In human words, what does the algorithm do? A docstring answering those basic questions would be helpful. A series of concise unit tests would be great.
I run the function to have a look at the output:
>>> text = "The weather is not good."
>>> result = negate_sequence(self=None, text=text)
>>> print(result)
['the', 'weather', 'the weather', 'is', 'weather is', 'the weather is', 'not', 'is not',
'weather is not', 'not_good', 'not not_good', 'is not not_good']
This doesn't ring a bell with me, so I stopped trying to understand the purpose.
Avoid stateful loops
Iteration i is coupled to iteration i-1 by the negation variable, this makes the logic hard to understand and error prone. If you work on bigrams/trigrams, I'd create a list of bigrams/trigrams and iterate over the tuples. This decouples the iterations.
Breakup long functions
This has almost endless benefits, as a starting point see this article. Some possibilities:
- Have the text broken up into alllowercase and without punctuation by
extract_words(text) - Have the list of trigrams created by
make_trigrams(words) - Inspect the trigrams by
process(trigrams) - If needed, have some kind of
aggregate(results)
Once this is done, I guess we are much better to prepared to identify bugs and to further improve functionality.
Here are a few suggestions on how to improve the code:
State the intent###
What is the purpose of the function? What is the input, what is the output? In human words, what does the algorithm do? A docstring answering those basic questions would be helpful. A series of concise unit tests would be great.
I ran the function to have a look at the output:
>>> text = "The weather is not good."
>>> result = negate_sequence(self=None, text=text)
>>> print(result)
['the', 'weather', 'the weather', 'is', 'weather is', 'the weather is', 'not', 'is not',
'weather is not', 'not_good', 'not not_good', 'is not not_good']
This doesn't ring a bell with me, so I stopped trying to understand the purpose.
Avoid stateful loops
Iteration i is coupled to iteration i-1 by the negation variable, this makes the logic hard to understand and error prone. If you work on bigrams/trigrams, I'd create a list of bigrams/trigrams and iterate over the tuples. This decouples the iterations.
Breakup long functions
This has almost endless benefits, as a starting point see this article. Some possibilities:
- Have the text broken up into all lowercase and without punctuation by
extract_words(text) - Have the list of trigrams created by
make_trigrams(words) - Inspect the trigrams by
process(trigrams) - If needed, have some kind of
aggregate(results)
Once this is done, I guess we are much better to prepared to identify bugs and to further improve functionality.
Here are a few suggestions on how to improve the code:
State the intent###
What is the purpose of the function? What is the input, what the output? In human words, what does the algorithm do? A docstring answering those basic questions would be helpful. A series of concise unit tests would be great.
I run the function to have a look at the output:
>>> text = "The weather is not good."
>>> result = negate_sequence(self=None, text=text)
>>> print(result)
['the', 'weather', 'the weather', 'is', 'weather is', 'the weather is', 'not', 'is not',
'weather is not', 'not_good', 'not not_good', 'is not not_good']
This doesn't ring a bell with me, so I stopped trying to understand the purpose.
Avoid stateful loops
Iteration i is coupled to iteration i-1 by the negation variable, this makes the logic hard to understand and error prone. If you work on bigrams/trigrams, I'd create a list of bigrams/trigrams and iterate over the tuples. This decouples the iterations.
Breakup long functions
This has almost endless benefits, as a starting point see this article. Some possibilities:
- Have the text broken up into alllowercase and without punctuation by
extract_words(text) - Have the list of trigrams created by
make_trigrams(words) - Inspect the trigrams by
process(trigrams) - If needed, have some kind of
aggregate(results)
Once this is done, I guess we are much better to prepared to identify bugs and to further improve functionality.