Return value directly from the try block and avoid a variable

[abhinav-upadhyay]

Avoid another temporary variable assignment by directly returning the value from the try block. The finally block will be executed nevertheless.

[Carreau]

I'm personally -1 as when try/except/finally blocks grows, we get return statement both in finally and in try, then the behavior of the block of code start to be unexpected. Indeed, even if finally will alway be runned (and the return), when you see a returnin try you usually assume the code will not go further and forget to check the finally.

[abhinav-upadhyay]

I think that's an invalid assumption to make. This behavior that the return statement in the finally block supersedes the return statement in try/except block is consistent in other languages like Java as well and I guess most of the programmers understand it.

For the case when the length of the try/except/block grows and it becomes complex to keep track, I think there should be tests to test the behavior in such cases, i.e. whether the correct value is being returned when an exception is raised, and when it is not (my 2 cents) :-)

Apart from that, I think the given patch fixes a potential bug. Consider the following code (from lib/latextools.py)

    try:
        with open(outfile, "rb") as f:
            bin_data = f.read()
    finally:
        shutil.rmtree(workdir)
    return bin_data

If an exception occurs while opening outfile then the code execution moves directly to the finally block and then comes to the return statement. At this point the bin_data object doesn't exist and a run time error will be raised.

[Carreau]

This behavior that the return statement in the finally block supersedes the return statement in try/except block is consistent in other languages like Java as well and I guess most of the programmers understand it.

I do not disagree with what id does and wether it is logic or not. I tend to take care about what people will think the block of code does when reading it in a few years.

The second point seem valid though, would there be a way to auto test that ?

[takluyver]

The behaviour of a return statement in a finally block might be standard, but it's not obvious - when we're reading code, we'll generally assume that the first return statement it reaches is what defines the return value, and finally blocks are essentially cleanup.

In general, we want both readable code and tests, not one or the other. Tests won't pick up all problems.

In the example you give, if an exception is raised, the code in the finally block will run, and then the exception will go up the stack, since there's nothing there to catch it. It wouldn't reach the return statement, if I'm reading it correctly.

I'm inclined to say, again, that we'd accept new code written either way, but we're not interested in changing things like this without a fairly clear rationale.

[abhinav-upadhyay]

On Sun, Oct 6, 2013 at 11:35 PM, Matthias Bussonnier <
notifications@github.com> wrote:

This behavior that the return statement in the finally block supersedes
the return statement in try/except block is consistent in other languages
like Java as well and I guess most of the programmers understand it.

I do not disagree with what id does and wether it is logic or not. I tend
to take care about what people will think the block of code does when
reading it in a few years.

The second point seem valid though, would there be a way to auto test that
?

On seconds thoughts I think it is a bit difficult to reliably test this,
since this is really about testing the inner mechanics of the function
rather than unit testing the expected return value on a specific set of
input parameters.

[abhinav-upadhyay]

On Sun, Oct 6, 2013 at 11:36 PM, Thomas Kluyver notifications@github.comwrote:

The behaviour of a return statement in a finally block might be standard,
but it's not obvious - when we're reading code, we'll generally assume that
the first return statement it reaches is what defines the return value, and
finally blocks are essentially cleanup.

I agree. But I am having a hard time thinking about a scenario where one
would have a return statement both in a try block as well as finally, in
which case the reader might be mislead.

In general, we want both readable code and tests, not one or the other.
Tests won't pick up all problems.

In the example you give, if an exception is raised, the code in the
finally block will run, and then the exception will go up the stack, since
there's nothing there to catch it. It wouldn't reach the return statement,
if I'm reading it correctly.

Right, I missed that part. The exception will be passed up in the stack,
but at least in this case, none of the users of this function are trying to
catch the exception.
examples/core/display.py: line 27
extensions/symprinting.py: line 61, line 73
lib/latextools.py: line 202

I'm inclined to say, again, that we'd accept new code written either way,
but we're not interested in changing things like this without a fairly
clear rationale.

I don't think I had any clear objective when sending the PR. I was going
through the code and found few things which I thought could be improved.
This was before you pointed me to the quickfix issues. I will go through
them and see if I can do something more useful :)

[takluyver]

Thanks.

In general, ret = ...; return ret is unnecessary, and we might accept a PR getting rid of it in simple cases. But these are kind of borderline simple cases. I'm +0 on merging this one.

[ellisonbg]

I am +0 as well.

[takluyver]

OK, merging this one, just to clear it up. If the finally blocks get more complex, they can be changed back if it seems appropriate.