chadtmiller.com/review-demo
A sample repository showing a pull with a review and line comments
Show the most common words too
Open Nice addition. A few things before it lands: the file is never closed, and the count is now required where it used to be optional.
wc.pyline 12OutdatedResolved
def top_words(text, n=5): | |
words = [w.lower() for w in text.split()] | |
counts = Counter(words) | |
return sorted(counts.items(), key=lambda kv: kv[1], reverse=True)[:n] |
Counter.most_common(n) does this sort and slice for you:
return counts.most_common(n)
wc.pyline 17Outdated
| |
def main(): | |
path = sys.argv[1] | |
n = int(sys.argv[2]) |
This fails with an IndexError when no count is given, which the old usage in the README did. Could it default to 5?
n = int(sys.argv[2]) if len(sys.argv) > 2 else 5
Good catch. I'll default it to 5 and keep the README's old example working.
wc.pylines 18 to 19Outdated
f = open(path) | |
text = f.read() |
The file is never closed now. The with open(path) as f: from before was better here.
wc.pyline 10OutdatedResolved
| |
| |
def top_words(text, n=5): | |
words = [w.lower() for w in text.split()] |
Punctuation stays attached, so word and word. count apart. Fine for now, maybe worth a follow-up.
This adds top_words and an optional second argument to wc.py. After the count, it prints the N most common words, lowercased, with N defaulting to 5. The README is updated to match. The logic is correct, and the repository has no tests or AGENTS.md to satisfy.
Two small things you could fix later, neither of which blocks the merge:
- Words are split on whitespace only, so "word," and "word" count as different words.
- A non-integer N raises a raw
ValueErrortraceback. A negative N doesn't error either:most_commonslices off the end of the list, so you get odd output.
1 approval
After the count,
wc.pynow lists the file's most common words, as many as the second argument asks for.