Skip to content

Finished text mining project!#2

Open
arpanrau wants to merge 2 commits intosd16fall:masterfrom
arpanrau:master
Open

Finished text mining project!#2
arpanrau wants to merge 2 commits intosd16fall:masterfrom
arpanrau:master

Conversation

@arpanrau
Copy link

@arpanrau arpanrau commented Oct 2, 2016

No description provided.

Copy link

@branchwelder branchwelder left a comment

Choose a reason for hiding this comment

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

Overall, good job. The code is clean and well laid out. The main theme is that you might actually leave too many comments, which can end up detracting from the readability of your code if not careful.

#congress democrats and republicans

#imports pattern web
from pattern.web import *

Choose a reason for hiding this comment

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

Although this isn't technically wrong, importing * can cause conflicts when libraries have functions with the same names. I would recommend instead importing some_library and then using some_library.somefunction(), or importing only the functions that you need with from some_library import some_function.

"""Finds sentiment of a given article on wikipedia.
accepts string searchterm where searchterm is the title of a wikipedia article.
Returns vector of (sentiment, objectivity)
where sentiment is between -1.0 and 1.0 and objectivity is between 0 and 1"""

Choose a reason for hiding this comment

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

I'm glad you wrote a docstring, it's really good practice. :)

#imports pattern web
from pattern.web import *
#imports pattern natural language
from pattern.en import *

Choose a reason for hiding this comment

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

Same comment as above.

#surprisingly some congressmen don't have wikipedia articles.
return (0,0) #returns no wikipedia sentiment if error is thrown
#(wikipedia article does not exist)

Choose a reason for hiding this comment

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

This is very clear. One main comment: there is such a thing as over-commenting. If you find yourself writing inline comments for every line, you're probably being too verbose. Just as a general note, you can assume that whoever is reading your code will have at least a passing familiarity with the language you're writing in - which means you don't have to comment that you are assigning an empty list to a variable. There is a point where some comments don't add any additional understanding, so it's best to just not write them

senatorlist = senatorsections.tables[0]#pulls list of current senators in table form

#iterates thru the table and classifies article names with Democrats and Republicans
for i in range(len(senatorlist.rows)):

Choose a reason for hiding this comment

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

This is totally fine, but there is this awesome thing called enumerate() that gives you both the current index and the values of the thing you are iterating through. I only recently started using it in my code and it's great. Learn more here, if interested: http://book.pythontips.com/en/latest/enumerate.html





Choose a reason for hiding this comment

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

You have a lot of whitespace here. Generally, you leave one line between conditionals/while/for loops and other code, and two lines between functions and other code.

democrataverage[0] = democrataverage[0]/democratcount
democrataverage[1] = democrataverage[1]/democratcount

#Print final scores

Choose a reason for hiding this comment

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

Same comment as above. A lot of unneeded comments.

@arpanrau
Copy link
Author

New commit: Revisions for style (Less comments/ fixed whitespace)

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