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

Completed MiniProject and WriteUp #63

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

Conversation

laurengulland
Copy link

Review on Reviewable

@kailevy
Copy link

kailevy commented Mar 18, 2016

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


histogram_analysis.py, line 7 [r1] (raw file):
You could turn these into functions which take the url and title as parameters


histogram_analysis.py, line 66 [r1] (raw file):
How do you choose to return 'rtrn'? It could be done with a boolean


histogram_analysis.py, line 98 [r1] (raw file):
These should be in a main() function


histogram_analysis.py, line 112 [r1] (raw file):
Instead of doing this with separate for loops:

plays = [top15_hamlet, top15_romeo...]
for play in plays:
    for word in play:
        for all_words.append(word)

histogram_analysis.py, line 144 [r1] (raw file):
What is this for?3


SoftDes MiniProject 3 - Text Mining and Analysis Write Up-1.pdf, line 0 [r1] ([raw file](https://github.com/sd16spring/textmining/blob/82d064c8c38e3febea98a3ba88b14ceeb487567b/SoftDes MiniProject 3 - Text Mining and Analysis Write Up-1.pdf#L0)):
Try matplotlib for visualization next time.


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