-
Notifications
You must be signed in to change notification settings - Fork 16
Mini Project 3 #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
@@ -1,3 +1,12 @@ | |||
# TextMining | |||
To run this program you will need to download the GitHub repository as well as the Natural Language Toolkit (NKTL), the requests package and the Vader sentiment analysis package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are just generally a few typos in this file, and this sentence is repeated at the bottom. I would suggest putting the lines that should go in the command line between `` because that's good markdown formatting. (ie: pip install vaderSentiment <= would look like => pip install vaderSentiment
)
minebooks2.py
Outdated
mystring = mystring.replace('-', '') | ||
mystring = mystring.replace('\"', '') | ||
mystring = mystring.replace(',', '') | ||
return mystring.lower() #makes everything lower case words like "The" and "the" are the same |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a cleaner way to do this with a for loop. Make a list of all the characters you want to eliminate and iterate through that list rather than writing down every single character as a distinct line.
minebooks2.py
Outdated
|
||
""" | ||
|
||
return( list(set(list(set(list1).intersection(list2))).intersection(list3))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor detail, but there's probably a cleaner way to do this.
minebooks2.py
Outdated
tragedies.append('Cymbeline.pickle') | ||
histories = ['King_Henry_IV.pickle'] | ||
histories.append('King_John.pickle') | ||
histories.append('King_Richard_II.pickle') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused about why you appended these strings instead of just writing them out initially.
Do this:
tragedies = ['Antony_and_Cleopatra.pickle', 'Coriolanus.pickle', 'Cymbeline.pickle']
instead of:
tragedies = ['Antony_and_Cleopatra.pickle']
tragedies.append('Coriolanus.pickle')
tragedies.append('Cymbeline.pickle')
Overall, this was a good project! I made a few stylistic notes. There are some areas where you could have used loops instead of writing out things manually. Your documentation was really good and your variable naming was on point. Very minor detail, but you had a few typos in your comments/ReadMe. It's always good to reread and double check your wording. |
No description provided.