[Perfect Numbers]: Perfect Square Test Cases

Hi! I’d like to report a test coverage gap in the Python track exercise (Perfect Numbers).

In short: I was able to submit a solution that does not handle all cases correctly, yet it still passed the test suite. The root cause is that the tests appear to miss some key perfect-square cases.

My approach was to iterate divisors only up to √n and use divisor pairs: if i divides n, then n//i also divides n. For any divisor pair (i, n//i), at least one of them is ≤ √n, so scanning up to √n is sufficient to discover all divisor pairs.

However, I wrote a loop like this:

limit = math.sqrt(number)
for i in range(1, math.ceil(limit)):
    ...

In Python, the upper bound of range is exclusive. When number is a perfect square, sqrt(number) is an integer k, but k is not included in the loop, so the factor k is missed (and for perfect squares it should be counted exactly once). This can change the classification result.

Counterexample: 196 = 14^2
Proper divisors of 196 are 1, 2, 4, 7, 14, 28, 49, 98, which sum to 203. Since 203 > 196, 196 should be classified as abundant.
If the square-root factor 14 is omitted, the sum becomes 203 − 14 = 189, which would incorrectly classify 196 as deficient.

Suggestion: add at least one perfect-square test case that exposes this issue (e.g., 196), or add a dedicated test for perfect squares, to prevent incorrect implementations from passing.

The problem spec contains test case 4 which is a perfect square. If a perfect square test is needed, does that suffice?

1 Like

No.

With 4:
correct logic: 1+2 < 4, so DEFICIENT
incorrect logic: 1 < 4, so DEFICIENT

With 196:
correct logic: 1+2+4+7+14+28+49+98 > 196, so ABUNDANT
incorrect logic: 1+2+4+7+28+49+98 < 196, so DEFICIENT

2 Likes

So the ask is to add a perfect number which is not deficient (either perfect or abundant)? That sounds good to me. I’ll give one vote (of the required three maintainer votes) for this.

1 Like

With 36:
correct logic: 1+2+3+4+6+9+12+18 > 36, so ABUNDANT
incorrect logic: 1+2+3+4+9+12+18 > 36, so ABUNDANT

The ask is to add a perfect square test case, where including or not including the square root would change the result.

I’m also a +1 for adding 196.

2 Likes

And a +1 from me as well.

Would you like to add that test case to the canonical data?

1 Like

Yes I would.

I’d like to add a bit of personal context for why I care about this coverage. When I was learning C in my first year, I struggled with the classic “sum of proper divisors of n” task as well. I initially looped all the way up to n, then realized I shouldn’t include n itself. Later, after printing the divisor list, I noticed all proper divisors are ≤ n/2 and changed the upper bound to n/2. After submitting, I was told there was still room for optimization. Eventually, with a small derivation, I understood the key idea: it’s enough to iterate up to √n and add divisors in pairs, which is both correct and significantly faster. For me, that was a valuable lesson about the trade-off between correctness and time/space complexity.

I also noticed that many community solutions use a straightforward range(1, n) approach. That will of course work for all inputs, including 196, and it avoids pitfalls like accidentally missing √n. However, it performs a lot of unnecessary checks and is essentially trading time for simplicity. Precisely because learners often try to optimize further to a √n approach, I think the test suite should enforce correctness on the key edge cases; otherwise a flawed √n implementation can slip through, especially if the learner doesn’t store divisors, print intermediate results, or debug step by step. They may never notice that the square-root factor was missed.

So I’d suggest adding at least one perfect-square test where missing √n changes the classification, such as 196. This would reliably catch the common bug and help learners write solutions that are both efficient and fully correct.

3 Likes

Go for it :slight_smile: Please link to this thread in the PR and then drop the PR link in this thread when you’re ready.

Sounds good to us!

2 Likes

Hi! Sorry, this is my first time doing this, so I just want to confirm the workflow.

My understanding is:

  1. Fork the problem-specifications repo and create a new branch.
  2. Add a new test case to exercises/perfect-numbers/canonical-data.json for number = 196 with expected = "abundant" (keeping the existing property name consistent with the file), like:
{
  "uuid": "",
  "description": "perfect square abundant number",
  "property": "classify",
  "input": { "number": 196 },
  "expected": "abundant"
}

Also, I noticed canonical-data.json requires a UUID for each test case. Is it okay if I generate a new UUIDv4 (e.g. via python -c "import uuid; print(uuid.uuid4())") and use it for this added test?

I’ll also link back to this forum thread in the PR description, as requested.

Yup. Pretty much.

We generally use the bin/configlet uuid command to generate a UUID4 but it doesn’t really matter how it’s generated.

After you push the commit to your branch, you should be able to create a pull request (PR) to continue your change back to the exercism/problem-specifications repo. You should do that which would give you the PR link we can use.

See also configlet uuid | Exercism's Docs

PR: Perfect Numbers: add perfect-square abundant test (196) by wesleywang513 · Pull Request #2620 · exercism/problem-specifications · GitHub

6 Likes

This has been merged.

1 Like

Thanks everyone for the review and for merging this! This was my first contribution to an open-source project, and my first PR accepted in a larger official repo. Really appreciate the guidance and the process.

4 Likes