Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pretty print bot exceptions #846

Open
Debilski opened this issue Feb 4, 2025 · 6 comments
Open

Pretty print bot exceptions #846

Debilski opened this issue Feb 4, 2025 · 6 comments

Comments

@Debilski
Copy link
Member

Debilski commented Feb 4, 2025

When we encounter an exception in the move function, we catch it in team.py and then manually print it with traceback.print_exc(). Modern Python versions do this much nicer with colour so, at the very least, we should change this line to:

traceback.print_exception(sys.exception(), limit=None, file=None, chain=True, colorize=True)

This will also make the exception easier to see in the terminal (assuming that a typical bot implementation is going to print hundreds of lines of black and white noise).

We could also opt for using rich here, which would be even fancier. It also comes with a neat addition in form of the show_locals flag that will list all local variables with their current values (super helpful, I’d say!):

console.print_exception(show_locals=True)

Unfortunately, we would only need this feature one frame above the current location; by default this will also print all variables that are currently defined in the Team.get_move function itself, which does make things rather confusing.

It seems to be possible to exclude things but excluding the whole of Pelita might also not be a good idea: https://rich.readthedocs.io/en/stable/traceback.html#suppressing-frames

@Debilski
Copy link
Member Author

Debilski commented Feb 4, 2025

We can avoid the messy stack trace by wrapping the call in another function that does not have as many local variables.

Example:

TEAM_NAME="crashing"
def move(b, s):
    b.say("hello")
    0/(b.round - 4)

    return b.position

Coloured Python stack trace:

Image

Rich:

Image

@otizonaizit
Copy link
Member

while I agree that it would be nice to print the colors starting from Python 3.13, I am not convinced about using rich and manipulate the exceptions in any way. After all pelita is used for teaching Python, so I think reading the exceptions in the way Python prints them is part of the things one has to learn.
I used to like the rich style, which is similar to what pytest does, but I have come to the conclusion that the format is too verbose and I struggle scrolling up and down in the terminal to find the spot that I should care about. But this is tangent to the main argument: it is not a matter of taste, I think that there is some value in leaving the exceptions untouched.

@Debilski
Copy link
Member Author

Debilski commented Feb 5, 2025

I wouldn’t call it ‘leaving the exceptions untouched’, given that we have to manually opt in for the Python 3.13 colours, but that’s also not the point of course.

If it weren’t for the local variables, I would also say that Python does it good enough. My colour argument is mostly about finding the exception easily beneath long lines of logging. (And if this is still a problem with Python 3.13 style errors then there are other ways to solve this, too.)

But having the local variables there could be a real advantage and timesaver. If you have some error in the 200th round, do you really want to litter your code with print statements, run it again and then wait 2 minutes for it to print it all and raise the exception again? (Obviously, this approach may still be needed, but perhaps not in all situations then.)

(Ignoring the debugger here because I think it is a different thing and also not great to use without a gui.)

@otizonaizit
Copy link
Member

ok, so maybe we need some more compelling examples? The screenshot you posted above does not even show that b.round == 4, which is the main problem with the code and the direct cause of the exception. At least for this particular example it doesn't seem to help much. But again, we are bike-shedding here.

The debugger is not great to use, but more or less perfect for exactly this use case: print things in the local name space. We want to teach people the concept of the debugger, that is why we have a demo for a debuggable bot.

@Debilski
Copy link
Member Author

Debilski commented Feb 5, 2025

It shows repr(b), which should indeed be fixed by us to be a little more useful in this situation (without being overwhelming). But it is a great hint, because indeed what we also should be adding is line saying ‘exception occurred for blue team: round 4, bot 1. Game was started with --seed 42.’ prior to printing the stack trace.

The problem with the debugger is that you have to engage it manually. What this context tells you is when you need to activate it. Without context, you are entering the debugger in every step round 1 and have to press c (and l) forever. But when you know that the exception occurs when some list is empty, you can set your breakpoint in a much more sensible place, which probably makes the debugger also much more convincing.

@otizonaizit
Copy link
Member

It shows repr(b), which should indeed be fixed by us to be a little more useful in this situation (without being overwhelming). But it is a great hint, because indeed what we also should be adding is line saying ‘exception occurred for blue team: round 4, bot 1. Game was started with --seed 42.’ prior to printing the stack trace.

Yes, this is a good idea, that we should address independently of the discussion about rich or not rich. It is not going to be so easy though, because just printing everything in bot is going to mess up the terminal even more, think of bot.walls, bot.food, etc...

The problem with the debugger is that you have to engage it manually. What this context tells you is when you need to activate it. Without context, you are entering the debugger in every step round 1 and have to press c (and l) forever. But when you know that the exception occurs when some list is empty, you can set your breakpoint in a much more sensible place, which probably makes the debugger also much more convincing.

sure, agreed. That is why the example above is not a good example. It does not tell you that the problem occurs at round 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants