Lately, I've been, I've been losing sleepDreaming about the things that we could beBut baby, I've been, I've been praying hard,Said, no more counting dollarsWe'll be counting stars, yeah we'll be counting stars(One Republic - Counting Stars)The 2nd Monitor is known to be a quite star-happy chat room, but exactly how many stars is there? And who are the most starred users? I decided to write a script to find out.I chose to write this in Python because:I've never used Python before@200_success used it and it didn't look that hardI found Beautiful Soup which seemed powerful and easy to useThe script performs a number of HTTP requests to the list of starred messages, keeps track of the numbers in a dict and also saves the pure HTML data to files (to make it easier to perform other calculations on the data in the future, and I had the opportunity to learn file I/O in Python).To be safe, I included a little delay between some of the requests (Don't want to risk getting blocked by the system)Unfortunately there's no way to see which user starred the most messages, but we all know who that is anyway...The code:from time import sleepfrom bs4 import BeautifulSoupfrom urllib import requestfrom collections import OrderedDictimport operatorroom = 8595 # The 2nd Monitor (Code Review)url = 'http://chat.stackexchange.com/rooms/info/{0}/the-2nd-monitor/?tab=stars&page={1}'pages = 125def write_files(filename, content):    with open(filename, 'w', encoding = 'utf-8') as f:        f.write(content)def fetch_soup(room, page):    resource = request.urlopen(url.format(room, page))    content = resource.read().decode('utf-8')    mysoup = BeautifulSoup(content)    return mysoupallstars = {}def add_stars(message):    message_soup = BeautifulSoup(str(message))    stars = message_soup.select('.times').pop().string    who = message_soup.select(.username a).pop().string    # If there is only one star, the `.times` span item does not contain anything    if stars == None:        stars = 1    if who in allstars:        allstars[who] += int(stars)    else:        allstars[who] = int(stars)for page in range(1, pages):    print(Fetching page {0}.format(page))    soup = fetch_soup(room, page)    all_messages = soup.find_all(attrs={'class': 'monologue'})    for message in all_messages:        add_stars(message)    write_files({0}-page-{1}.format(room, page), soup.prettify())    if page % 5 == 0:        sleep(3)# Create a sorted list from the dict with items sorted by value (number of stars)sorted_stars = sorted(allstars.items(), key=lambda x:x[1])for user in sorted_stars:    print(user)The results, you ask? Well, here they are: (Spoiler warning!) (I'm only showing those who have \$> 50\$ stars here, to keep the list shorter) ('apieceoffruit', 73) ('ChrisW', 85) ('Edward', 86) ('Yuushi', 93) ('Marc-Andre', 98) ('nhgrif', 112) ('amon', 119) ('James Khoury', 126) ('Nobody', 148) ('Jerry Coffin', 150) ('BenVlodgi', 160) ('Donald.McLean', 174) ('konijn', 184) ('200_success', 209) ('Vogel612', 220) ('kleinfreund', 229) ('Corbin', 233) ('Morwenn', 253) ('skiwi', 407) ('lol.upvote', 416) ('syb0rg', 475) ('Malachi', 534) ('retailcoder', 749) (Mat's Mug, 931) ('Simon Andr Forsberg', 1079) ('Jamal', 1170) ('The Mug with many names', 2096) (Mat's Mug, retailcoder and lol.upvote is the same user) ('rolfl', 2115)It feels strange to do .pop() to fetch data from the select soup, is there another approach available here? However, as this is the first time I'm using Python, any comments are welcome.
We'll be counting stars
python;python 3.x;stackexchange;web scraping;beautifulsoup
That's a beautiful excuse for writing code, and the final product is quite nice as well. First of all, congrats to your Java-ridden mind for not forcing classes into Python where they aren't needed. You import but do not use OrderedDict and operator. Remove unused imports. It is customary to write your code in a way that allows it to be used as either a module, or a script. For this, the if __name__ == '__main__' trick is used:if __name__ == '__main__':    # executed only if used as a script You declare a few variables like room, url, and pages up front. This hinders code reuse:The room variable should not be declared up front as a global variable, but in your main section. From there, it can be passed down through all functions.The url specifically but unnecessarily mentions the-2nd-monitor. This isn't harmful, but unnecessary as only the ID is relevant. Furthermore, url is an awfully short name for such a large scope. Something like star_url_pattern would be better  except that global constants should be named all-uppercase:STAR_URL_PATTERN = 'http://chat.stackexchange.com/rooms/info/{0}/?tab=stars&page={1}'Reserve plural names for collections. pages should rather be page_count. But wait  why are we hardcoding this rather than fetching it from the page itself? Just follow the rel=next links until you reach the end. That last idea could be implemented with a generator function. A Python generator function is similar to a simple iterator. It can yield elements, or return when exhausted. We could build such a generator function that yields a beautiful soup object for each page, and takes care of fetching the next one. As a sketch:from urllib.parse import urljoindef walk_pages(start_url):    current_page = start_url    while True:        content = ... # fetch the current_page        soup = BeautifulSoup(content)        yield soup        # find the next page        next_link = soup.find('a', {'rel': 'next'})        if next_link is None:            return        # urljoin takes care of resolving the relative URL        current_page = urljoin(current_page, next_link.['href']) Please don't use urllib.request. That library has a horrible interface and is more or less broken by design. You might notice that the .read() method returns raw bytes, rather than using the charset from the Content-Type header to automatically decode the content. This is useful when handling binary data, but a HTML page is text. Instead of hardcoding the encoding utf-8 (which isn't even the default encoding for HTML), we could use a better library like requests. Then:import requestsresponse = requests.get(current_page)response.raise_for_status()  # throw an error (only for 4xx or 5xx responses)content = response.text  # transparently decodes the content Your allstars variable should not only be named something like all_stars (notice the separation of words via an underscore), but also not be a global variable. Consider passing it in as a parameter to add_stars, or wrapping this dictionary in an object where add_stars would be a method. I don't quite understand why you write each page to a file. I suspect this was intended as a debugging help, but it doesn't add any value to a user of that script. Instead of cluttering the current working directory, make this behavior optional. Do not compare to None via the == operator  this is for general comparison. To test for identity, use the is operator: if stars is None. Sometimes, it is preferable to rely on the boolean overload of an object. For example, arrays are considered falsey if they are empty.Talking is easy,coding is hard.Is this refactoringequally bad?import timefrom bs4 import BeautifulSoupimport requestsfrom urllib.parse import urljoinSTAR_URL_TEMPLATE = 'http://chat.{0}/rooms/info/{1}/?tab=stars'def star_pages(start_url):    current_page = start_url    while True:        print(GET {}.format(current_page))        response = requests.get(current_page)        response.raise_for_status()        soup = BeautifulSoup(response.text)        yield soup        # find the next page        next_link = soup.find('a', {'rel': 'next'})        if next_link is None:            return        # urljoin takes care of resolving the relative URL        current_page = urljoin(current_page, next_link['href'])def star_count(room_id, site='stackexchange.com'):    stars = {}    for page in star_pages(STAR_URL_TEMPLATE.format(site, room_id)):        for message in page.find_all(attrs={'class': 'monologue'}):            author = message.find(attrs={'class': 'username'}).string            star_count = message.find(attrs={'class': 'times'}).string            if star_count is None:                star_count = 1            if author not in stars:                stars[author] = 0            stars[author] += int(star_count)        # be nice to the server, and wait after each page        time.sleep(1)    return starsif __name__ == '__main__':    the_2nd_monitor_id = 8595    stars = star_count(the_2nd_monitor_id)    # print out the stars in descending order    for author, count in sorted(stars.items(), key=lambda pair: pair[1], reverse=True):        print({}: {}.format(author, count))