There are a number of tests that involve negative numbers, but many of the “Dig Deeper” examples rely on isdigit() to check for a valid operand and that will fail with negative numbers since the minus sign is not a digit. The “Dig Deeper” solutions that fail in this way should be updated, or the problem specification and tests modified to explicitly exclude negative numbers.
Furthermore, there are no tests for zero operands, which allows implementations that fail to distinguish between a missing operand and a zero. Either some zero operand tests should be added or the problem specification should explicitly require only positive integers in the questions.
Since I wrote the Wordy approaches, I will go in sometime this week and re-test the examples. When the approaches were written, all of them were tested extensively — but something must have changed, or errors must have slipped through. I’ll dig in in the next few days.
As for problem-specifications and new tests — can I ask you to open a new thread for that discussion? Re-doing examples on the Python track is one thing, but proposing new tests for problem specifications requires discussion and sign-off from multiple track maintainers.
I do have a concern tho. Just messed around with a few lines in the online editor, and it appears that the runner isn’t actually testing anything. Not sure what is up, but I think we have an issue…
Looking closer, “Approach: Import Callables from the Operator Module” passes “What is -5?” because the following lines are superfluous and misleading:
if question.isdigit():
return int(question)
The actual result is returned by return equation[0] at the end of the function. This does not teach good coding practices.
“Approach: Dunder methods with __getattribute__” has a similar problem, and fails “What is -5?” with “ValueError: unknown operation”.
This is a demonstration of poor test coverage masking a coding error. You could argue that it was intended that the single operand “What is” case (and only that case!) does not accept negative numbers because that’s what the tests permit, but that sort of inconsistency should then be called out explicitly as a requirement in the problem specification, otherwise it’s just teaching bad practices. Or you could just respecify the problem as “positive integers only” and relax the tests.
It’s a difficult and fine line to be “exhausitive” and still teaching things.
In this particular case I agree with your assessment that:
either the instructions / test need updating to bound the input to “positive integers”
or the tests should be more extensive.
Personally, as I just re-did this exercise, in THIS instance, I think we should add a test case for negative numbers, but should update the content that it’s only whole numbers (integers).
My preference would be to remove the two lines called out above from “Approach: Import Callables from the Operator Module”, add a new test for “What is -42?” and correct the error in “Approach: Dunder methods with __getattribute__” as the least radical solution.
If we don’t want to fail any existing submissions that might previously have passed, then the only real consistent option is to change the specification to “positive integers only” and update all the other tests that use negative numbers. Then clean up the “Dig Deeper” examples to not bother (sometimes) checking for negative numbers.
I’m going to defer changing code examples until we agree on test changes, since changes to problem specifications will require that I update the exercises, regenerate the tests, and probably update the example solution as well as the dig deeper docs. Much less chaos that way.
