python icon indicating copy to clipboard operation
python copied to clipboard

[Error-Handling]: instructions are unclear, and wrong programs pass the tests

Open Stigjb opened this issue 5 years ago • 11 comments

I just did the error-handling exercise, and had several issues:

  • It was unclear at first what to do with inputs. Looking at the tests, it seemed that trying to convert the input to int was the right thing to try, but this is not mentioned in the readme or the stubs.
  • The test for whether a method throws an exception will pass any error that might occur, not just if it's intentionally raised:
def handle_error_by_throwing_exception():
    raise "(╯°□°)╯︵ ┻━┻"  # Raising a string is a TypeError, but passes the test
  • When returning a tuple, which element should be the result and which one the status? What should the result element be when it fails? No suggestion
  • No mention of advantages or disadvantages of the different kinds of error handling, or what is idiomatic Python.

Stigjb avatar May 14 '19 13:05 Stigjb

The problem with the test example is that the assertRaises methods delete the traceback of the exception before they return them. The closest this could probably get would be:

with self.assertRaisesWithMessage(Exception) as err:
    er.handle_error_by_throwing_exception()
err_type = err.exception.__class__.__name__
code = inspect.getsourcelines(er.handle_error_by_throwing_exception)[0]
self.assertIn(err_type, "".join(code))

However, that is limited to checking whether the exception class name is referenced somewhere in the entire function, and not just the statement that actually raised it.

The entire error-handling exercise is a bit problematic in its current implementation. It will likely be replaced with a proper Concept Exercise in v3.

cmccandless avatar Mar 08 '20 15:03 cmccandless

Yeah, this exercise needs a complete rethink.

My best thought is that the dummy file include something like this:

# do not change the UnhappyError, but be prepared to handle it below.
class UnhappyError(Exception):
    pass

def convert_unhappy_to_none(func, *args, **kwargs):
    # you may move the following line, but it _must_ appear in your function
    return func(*args, **kwargs)

def convert_unhappy_to_bool(func, *args, **kwargs):
    # you may move the following line, but it _must_ appear in your function
    return func(*args, **kwargs)

def convert_unhappy_to_error_code(func, *args, **kwargs):
    # you may move the following line, but it _must_ appear in your function
    return func(*args, **kwargs)

def convert_unhappy_to_typeerror(func, *args, **kwargs):
    # you may move the following line, but it _must_ appear in your function
    return func(*args, **kwargs)

The test file can then import the UnhappyError and generate mocks that will raise it as a side_effect in appropriate circumstances. Then we'd just need tests to validate that everything worked when no error was raised as well as when an UnhappyError was raised ... we can probably relax our usual habit of ignoring what's actually in an error message and simply write the exercise so the student MUST pass-through the message the tests put into the UnhappyError verbatim when converting to a new error type.

yawpitch avatar Mar 08 '20 16:03 yawpitch

If you are going to go that far, don't forget to include having to use the three types of raise statements. Also, you can monkey patch whatever you want using @patch so the exception can be generated without having to pass in a specific function to be called.

Thinking about this, it could almost be step 2 of another exercise. For example, a tour of the built-in functions or types. In the example, step one is doing the conversions and calls with normal input (e.g. turn "123" into 123 or bool(None) == False), and this step two is the same but with bad calls or raised exceptions (int("football") or set(123) raises an exception) to teach people how to handle them before things get too complicated. I'm not married to the idea of the built-ins, but it shows the general concept of extending a previous exercise.

I noticed this yesterday! I understand that the v3 version of the track will have a new version of this exercise, but could we fix this in the interim with more descriptive hints?

As it stands the student is left without a real direction to move to pass the tests, without just trying stuff out. If the intent is to show how to use with and raise, could we give them hints as to what we want in each function?

Edit - Glad to submit a PR if that makes sense for a short term fix.

morganpartee avatar Jan 18 '21 18:01 morganpartee

@morganpartee -- Thanks so much for volunteering to help clarify this exercise! Since we are close to launching a beta of V3 (within the next two months), and it looks like this exercise needs a complete re-work, we've decided to mark it as deprecated (which will remove it from the website for the Python track) until we can get both the V3 error-handling concept exercises completed and the re-write of this exercise completed.

We would love if you'd like to help with either the V3 concept exercises or a re-write of this practice exercise...we just don't want you to do a bunch of "short-term fix" work that won't get used. If you would like to take on either the V3 exercises or a re-write of this one, please reach out and let us know. Thanks again.

BethanyG avatar Jan 18 '21 19:01 BethanyG

Thanks @BethanyG! I'll definitely join in on v3. I'm behind the curve on developments there, but I'm glad to help.

morganpartee avatar Jan 18 '21 20:01 morganpartee

@BethanyG do we want to close this issue (and mark as resolved by #2281), or leave it open as a reminder to address this exercise after requisite Concept Exercises are complete?

cmccandless avatar Jan 19 '21 18:01 cmccandless

@cmccandless -- I think this would be good to leave open as a reminder. Otherwise, it might get lost in the shuffle.

Ultimately, we want to log both the error-handling concept exercise issues and a re-write of this practice exercise as its own issue, but I don't have time atm to do so -- so I think the best bet is to leave this pending.

BethanyG avatar Jan 19 '21 19:01 BethanyG

This issue has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Feb 19 '21 01:02 github-actions[bot]

See also #2275 for additional considerations for the error handling concept cluster and set of exercises.

BethanyG avatar May 21 '22 22:05 BethanyG