Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I wrote a short script in Python 3 that connects to the Stack Exchange API, gets all questions on Programming Puzzles & Code Golf over the past two weeks, and determines the average number of questions per day as well as the average number of answers per question.

The number of questions per day is intended to match that on Area 51, which it does. Obviously it's much easier to just scrape Area 51 directly, but I wanted to figure it out myself for practice.

I'm not an expert with Python or with web APIs, so I was hoping you fine Code Review folks can help me improve my practices.

import requests, datetime, time

def seconds_since_epoch(dt):
    epoch = datetime.datetime(1970, 1, 1, tzinfo=datetime.timezone.utc)
    return int((dt - epoch).total_seconds())

today = datetime.datetime.now(datetime.timezone.utc)

params = {
    "site": "codegolf",
    "fromdate": seconds_since_epoch(today - datetime.timedelta(days=14)),
    "todate": seconds_since_epoch(today),
    "pagesize": 100,
    "page": 1
}

base_url = "https://api.stackexchange.com/2.2"

results = []

while True:
    req = requests.get(base_url + "/questions", params=params)
    contents = req.json()
    results.extend(contents["items"])
    if not contents["has_more"]:
        break
    if "backoff" in contents:
        time.sleep(contents["backoff"])
    params["page"] += 1

questions_per_day = len(results) / 14
answers_per_question = sum([q["answer_count"] for q in results]) / len(results)

print("Over the past 2 weeks, PPCG has had...")
print(round(questions_per_day, 1), "questions per day")
print(round(answers_per_question, 1), "answers per question")

My approach is to build the query using a dict and make the request to the API using the requests module. I set the page size to the maximum to reduce the number of requests made so that the daily quota isn't exhausted quite so fast.

The code is hosted on GitHub, should you want to fork and adapt it for your own purposes, assuming it isn't too terrible.

share|improve this question
    
Hi Alex :D nice code for a Python newbie ;) – cat Feb 18 at 22:54
    
@cat Thanks! I'm familiar enough with Python to be effective but I haven't used it consistently enough to be good at it. :P – Alex A. Feb 18 at 23:00
up vote 12 down vote accepted

Your seconds_since_epoch function has a built-in Python equivalent, datetime.timestamp.

Your namespaces would be cleaner if you did from datetime import datetime, timezone.

You use a base_url variable, but do not use urllib.parse.urljoin. Either use a hardcoded URL, or properly join the base URL with the fragment.

results is better named as questions.

In sum([q["answer_count"] for q in results]) the [] is superfluous and inefficient.

Instead of printing 3 times in a row create a multiline format string and print once.

You never create a function that returns the questions, and do not define a main function. I suggest printing in the main function, that calls a function that gets and returns the question information.


This is how I would program it:

import requests
import time
from datetime import datetime, timezone, timedelta


def get_question_info(site, start, stop):
    API_URL = "https://api.stackexchange.com/2.2/questions"
    req_params = {
        "site": site,
        "fromdate": int(start.timestamp()),
        "todate": int(stop.timestamp()),
        "pagesize": 100,
        "page": 1
    }

    questions = []
    while True:
        req = requests.get(API_URL, params=req_params)
        contents = req.json()
        questions.extend(contents["items"])

        if not contents["has_more"]:
            break
        req_params["page"] += 1

        if "backoff" in contents:
            time.sleep(contents["backoff"])

    return questions


def get_area51_estimate(site):
    now = datetime.now(timezone.utc)
    fortnight_ago = now - timedelta(days=14)
    questions = get_question_info(site, fortnight_ago, now)
    avg_questions = len(questions) / 14
    avg_answers = sum(q["answer_count"] for q in questions) / len(questions)
    return avg_questions, avg_answers


if __name__ == "__main__":
    msg = """Over the past 2 weeks, PPCG has had...
{:.1f} questions per day
{:.1f} answers per question"""
    print(msg.format(*get_area51_estimate("codegolf")))
share|improve this answer
    
In your third point, you are saying that the OP is not currently use that urljoin method, but they should/could be, right? I think you are missing some words in that sentence. – SirPython Feb 18 at 23:15
    
Did you put API_URL in all caps because it's constant? – Alex A. Feb 18 at 23:51
    
@AlexA. Correct. – orlp Feb 18 at 23:58

Code Style

import requests, datetime, time

Imports should not be on one line, as per PEP8

Your code doesn't meet the "too many characters on one line" rule of PEP8, however, you should avoid having stuff like this on one line:

epoch = datetime.datetime(1970, 1, 1, tzinfo=datetime.timezone.utc)

base_url:

You don't use the base_url for anything else adding /questions, so just add it to base_url as is. (and rename it url, then)

base_url + "/questions"

Magic Numbers:

What is 14? And why are you dividing it by the amount of results?

Magic numbers should be moved to another variable to increase readability.

len(results) / 14

Removing "site": "codegolf"

I mentioned this to you in chat, but if you do a quick input() for sitename you can expand this script out to any site supported by the API.


class structure:

Building on my point above, if you used a class structure, you can extract most of this logic into separate parts, and use a generator to fetch the next page instead of a incrementing the page param.

share|improve this answer
3  
If each import should be on a separate line, then what's the point of having the comma in Python syntax? :/ – Cᴏɴᴏʀ O'Bʀɪᴇɴ Feb 18 at 23:42
1  
I'm assuming it was a core language feature before PEP8 – Quill Feb 18 at 23:43
    
The comma is typically used when importing modules/methods/etc from a specific module. The "don't use commas" rule only applies when importing only the root module. It can be generalized to "each module gets its own import line". – Kevin Brown Feb 19 at 2:32
    
That makes sense @KevinBrown, thanks for the explanation :-) – Quill Feb 19 at 2:33
    
Small typo: "should be on one line" -> "should not be on one line" – pydsigner Feb 19 at 16:22

A small bit of ambiguity

params = {
    ...
}

...

while True:
    req = requests.get(base_url + "/questions", params=params)

Why not name your params variable something else? That last line is les readable than if you named the variable something like site_info, as one who isn't acquainted with the oddities of Python might distinguish the named argument from the variable easier. (As a programmer who frequents JavaScript, that line sure confused me. ¯\_(ツ)_/¯) Besides that, it's more readable, as your mind doesn't have to make the extra step to distinguish the variable fro the named parameter.

share|improve this answer
    
@SirPython Not that I know of, and I'm pretty in to ES6. But as Quill says, there apparently is... I'm not acquainted with it nonetheless. – Cᴏɴᴏʀ O'Bʀɪᴇɴ Feb 18 at 23:19
    
@CᴏɴᴏʀO'Bʀɪᴇɴ I apologize; I mistook it for another feature (the one where you can specify default values for parameters. – SirPython Feb 18 at 23:20
    
@SirPython Ah, I see. That makes sense, and is also what contributes to my own confusion about named parameters. ;) – Cᴏɴᴏʀ O'Bʀɪᴇɴ Feb 18 at 23:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.