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

New pull request #14

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

New pull request #14

wants to merge 3 commits into from

Conversation

feberhardt
Copy link

This is the new pull request for the feedback


>>>
"""
WORD = re.compile(r'\w+')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would caution against using all caps for variable names because some programs will interpret them differently.

text_7 = 'Moby Dick by Melville.txt'


Text_similarity(text_1, text_2, text_3, text_4, text_5, text_6, text_7)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should wrap these last lines of code in a final function and call that function at the end. On a more general note, Text_similarity works fine here because you're only dealing with 7 texts, but imagine if you had 20 or 100 texts to compare. You wouldn't want to write text_i over and over again. In the future, think about ways to make this more efficient.

@Elepert
Copy link

Elepert commented Oct 16, 2017

Overall, good job! Your documentation is good and I made a few stylistic comments. Your ReadMe should contain instructions on how to run your program, include any libraries you installed and the installation instructions, indicate which file to run to get your results, and have a link to your reflection.

I implemented all the annotations
@feberhardt
Copy link
Author

This is the implementation of the notes made by Emily

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.

None yet

2 participants