Skip to content
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

Rachel Yang Text Mining Project #47

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

RachelYang02
Copy link

Review on Reviewable

@LucyWilcox
Copy link

Review status: 0 of 63 files reviewed at latest revision, 6 unresolved discussions.


movie_subtitles.py, line 34 [r1] (raw file):
I'm a little confused, what's happening with hist here? What does it go on to do?


movie_subtitles.py, line 85 [r1] (raw file):
You can git rid of this stuff if you're not using it.


TM_ProjectWriteUp.pdf, line 0 [r1] (raw file):
Good discussion. Also next time please don't push every file (like all the .src, .txt, and .png), it's overwhelming!


word_cloud_2.py, line 50 [r1] (raw file):
👍


word_cloud_2.py, line 59 [r1] (raw file):
I don't really understand why you are using two list and a function from a different file. It seems like they should be here considering you are no longer using the other file to make word histograms.


word_cloud_2.py, line 63 [r1] (raw file):
If there's not a good reason for you to have two different lists of images here you shouldn't.


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