Skip to content

Conversation

@guokevin
Copy link

Review on Reviewable

@srli
Copy link

srli commented Mar 20, 2016

Review status: 0 of 8 files reviewed at latest revision, 12 unresolved discussions.


a discussion (no related file):
As a preface, please submit any writeups in PDF or Markdown format in the future, since we can't view .docx files directly in Github and have to download them.

Your code works well, and is very neatly written. The comments are great, so it was really easy to follow along your logic. There are areas where code was unnecessarily repeated, please take a look at my comments!

You talk about using DSU and NLP in your Project Overview, though neither seem to be implemented in the code. In the future, I would focus the overview more on the techniques actively used in the project.

All in all, nicely done!


TextMining.py, line 12 [r1] (raw file):
It doesn't appear that you're using any nltk imports in your code, so delete them from your import list for clarity


TextMining.py, line 32 [r1] (raw file):
A more descriptive variable name like word_histogram would make reading the code easier


TextMining.py, line 58 [r1] (raw file):
Since there is only one line where this function differs from the code in create_dictionary_list, I would suggest putting them together into one function.


TextMining.py, line 65 [r1] (raw file):
You can simplify this function using a for loop.

dict_list = []

for b in range(2,6):
book = open("Text"+str(b)+".txt")
book_text = book.read().lower()
book_text = re.sub('[^a-z]', ' ', book_text)
dict_list.append(create_dictionary(book_text.split())
book.close()

return dict_list


TextMining.py, line 76 [r1] (raw file):
dictionary is a fairly generic variable name. Something like word_histogram would be clearer here.


TextMining.py, line 118 [r1] (raw file):
dictionary.keys() will also give you all the keys of the dictionary in list form


TextMining.py, line 131 [r1] (raw file):
A different way to do this could be to use a sorted representation of dictionaries like this


TextMining.py, line 156 [r1] (raw file):
You can also directly access each dictionary in the list:

for dict in dictionary_list:


TextMining.py, line 168 [r1] (raw file):
Delete unused code for final turnins


TextMining.py, line 188 [r1] (raw file):
You use this same code in a previous function, try to eliminate repeated code for style


TextMining.py, line 219 [r1] (raw file):
Delete unused code for final turnin


Comments from the review on Reviewable.io

@srli
Copy link

srli commented Mar 20, 2016

Reviewed 8 of 8 files at r1.
Review status: all files reviewed at latest revision, 12 unresolved discussions.


Comments from the review on Reviewable.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants