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

Add files via upload #4

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

Add files via upload #4

wants to merge 4 commits into from

Conversation

jwen2
Copy link

@jwen2 jwen2 commented Oct 12, 2017

No description provided.

Copy link

@SeunginLyu SeunginLyu left a comment

Choose a reason for hiding this comment

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

Nicely Done! Your code is functional and is concise stylistically. But I feel like you can improve your documentation by adding more inline comments and clarifying some of your docstrings. I've also made minor stylistic suggestions for revision!


def gettext(url):
""" maybe modify this to incorporate other websites
if I have the time"""

Choose a reason for hiding this comment

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

This docstring does not explain what this function is about. Please refer to Oliver's GeneFinder solution. https://github.com/sd17fall/GeneFinder/blob/formatted/gene_finder.py

return requests.get(url).text

Christmas = gettext('http://www.gutenberg.org/cache/epub/46/pg46.txt')
#OliverTwist = gettext('http://www.gutenberg.org/ebooks/730.txt.utf-8')

Choose a reason for hiding this comment

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

Please remove comments for your final code

cleanedlist.append(word)
return cleanedlist

#print(len(cleanuplist(ATaleofTwoCities)))

Choose a reason for hiding this comment

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

remove comments

>>> cleanuplist('This project is so hard!')
['this', 'project', 'is', 'so', 'hard']
>>> cleanuplist('I need, a bunch, of !? doctest?')
['i', 'need', 'a', 'bunch', 'of', 'doctest']

Choose a reason for hiding this comment

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

nice unit test! 👍

cleanedlist = []
textlist = textlist.lower().split()
for word in textlist:
symbols = "-_=+[}{]:;?/.>,<?!@#$%^&*()|'"

Choose a reason for hiding this comment

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

This is a good way of implementing this function. I also suggest you look into regular expression if you want your code to be more concise! https://en.wikipedia.org/wiki/Regular_expression


#print(topNvalues(wordcounter(ATaleofTwoCities),5))

def uniquewordsused(s):

Choose a reason for hiding this comment

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

one way to make this function name more readable is by using camelcase convention like uniqueWordUsed https://sanaulla.info/2008/06/25/camelcase-notation-naming-convention-for-programming-languages/


def suffixdictionary(s):
""" Takes the premade dictionary key index and starts appending
suffixes to the list of values for each key

Choose a reason for hiding this comment

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

So what is this function returning? I think you can be more clear with your docstring.

@jwen2 jwen2 closed this Oct 18, 2017
@jwen2 jwen2 reopened this Oct 18, 2017
@SeunginLyu
Copy link

Nice, I see improvements in your documentation. I still suggest you to use the style https://github.com/sd17fall/GeneFinder/blob/formatted/gene_finder.py here for function documentation.

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