I don’t think anyone here is seriously suggesting removing for/else.
8 Likes
It was mentioned a few times, but it’s not part of this PEP so it doesn’t really need defending now.
1 Like
As an “insider” 
, let me clarify: it’s not being considered.
Guido designed Python. for/else was his idea to begin with. He’s sometimes joked that it’s the only wholly new idea he had.
Guido was impressed by that analysis in this PEP showed not just that the construct in question is rarely used, but does a wrong (unintended by the programmer) thing in a large majority of the cases it is used. It’s clearly a “bug magnet”, and to an extraordinary degree.
Guido did not go on to suggest getting rid of for/else too, but only that he’d like to see a similar kind of “real world” analysis done for that construct. I would too, for that matter, but already explained why I think it would be very much harder to do. So much harder that I doubt anyone will do it.
But, if they did, and discovered that over 70% of real-world uses were incorrect, ya, then the language has a real problem. One possible way of addressing that would be to deprecate for/else too.
But that’s all several layers of highly unlikely (according to me) hypotheticals away from where we are here.
It’s natural for topics to branch off into vaguely related areas. But any more talk of for/else here would probably justify splitting those off into a different topic. It’s emphatically not the issue in this topic, and has only weak purely hypothetical connections.
15 Likes
Thank you everyone, I’ve updated the PEP as suggested here and submitted it to the SC for consideration.
11 Likes
I’m not sure whether to reply to @steve.dower here or to their comment on the Github issue in the steering council repository. Doing so here as that seems most visible to all participants.
I’m specifically responding to:
This is a good candidate for linters to warn about. Not for the compiler in the runtime.
pylint has been warning about this for more than a year (see release notes of March 2023). ruff also has this as a message although I’m not familiar enough with the project to determine since when.
One of the projects that Irit found to be incorrectly using this pattern, actually uses ruff but still didn’t get the warning as they hadn’t enabled this particular message. I think this is a perfect example of why saying that linters should do this check doesn’t completely solve the issue. Other issues, besides linters implementing the check but users not turning on the check, include linters being out of date, linters being poorly implemented, etc etc.
I think the research by Irit and others clearly establishes that 1) the pattern is often misunderstood and 2) can be easily replaced by different patterns. Turning this into SyntaxWarning seems sensible and useful to me. It will help users prevent these issues, whereas the other proposed solution (ie., let linters do this) is clearly suboptimal.
I just randomly picked this example by looking at Irit’s Github profile, checking their recently opened issues and checking whether this project used a linter. This was the first issue I opened. The research doesn’t include a full list of projects misusing the pattern, or I couldn’t find it, so I can’t really do a full analysis on linter usage.
1 Like
Thanks, yes, this is a more appropriate place. My comment on the issue is specifically for the Steering Council, in case they don’t take the time to review the thread (and so assume that there’s complete consensus). I’m sure they’ll come and read this as a result, unless they have a strong reaction of their own to the PEP text, so your comment will be seen.
Which is a good argument for suggesting the message be on by default, but not, IMHO, a reason to bypass linters entirely and do it ourselves.
There’s certainly enough consensus here to go to ruff on behalf of the core team and say that this deserves an on-by-default warning.
5 Likes
Which is a good argument for suggesting the message be on by default, but not, IMHO, a reason to bypass linters entirely and do it ourselves.
I would generally agree, but just don’t really see what problem this pattern is solving. For me that is one of the most important conclusions of Irit’s research: all “valid” patterns are replaced with a trivial other pattern that avoids the pitfalls of the current one.
There’s certainly enough consensus here to go to ruff on behalf of the core team and say that this deserves an on-by-default warning.
At my day job we turn off all linter messages and only turn them on when somebody wants to spend the time to fix all issues, which you can imagine is almost never. I think it would still be good to indicate to ruff that this is a very useful message, but again that would likely not reach all audiences. (In fear of pinging them and involving them in a discussion they don’t want to get involved in, @charliermarsh this might be relevant for you and your team).
I’m not sure you can answer this, but would there be something that could convince you to support this proposal? For me the evidence of misuse and possibility of replacing it with a (in my opinion) clearer pattern is enough, but I’m wondering if there is other evidence that is currently lacking that could convince you.
I don’t have any problem with the evidence, it’s just the required action. Python has many millions of users and billions of lines of code - there’s rarely a percentage small enough to justify as “this won’t affect anyone”. So we should assume that people will be affected.
We know that compiler warnings have an outsized impact on users over maintainers, which is why DeprecationWarnings are off by default. So we should assume that primarily innocent users will be affected by a SyntaxWarning.
The approach that would convince me to support it is to create a new warning type and have it be off by default. But that wouldn’t actually convince me because there’s no way that’s worth the cost (which is a different reason), and I believe nobody else involved would prefer that way for the same reason.
But if there’s another approach that doesn’t bother users, that doesn’t bother maintainers who are using it correctly, and doesn’t bother anyone who chooses to suppress the bother, then I’m fine with it.
2 Likes
I didn’t publish the full list because I think it would turn the focus of the report from “a problem with the language” to “a shaming list of broken libraries”. I did publish the script I used to produce the list, so it is possible to replicate. I can also share my spreadsheet privately if anyone needs it for research purposes.
2 Likes
You can easily avoid that warning by overwriting the code
try:
...
finally:
# some code
as
try:
...
except:
# some code
raise
else:
# some code
This is an equivalent code and it has the same flaws as the original code if the cleanup code contains unconditional return/break/continue. And now you have a chance to take a fresh look at that code and rewrite it in a way that makes more sense. You can now have different code for except and else causes.
A user cannot. You’re making the argument for warning the maintainer via a linter, not for warning the end user via a compiler warning.
3 Likes
Why is it important to avoid the warning? We went for warning rather than error so that the program will continue to run. Is that not what SyntaxWarning is for?
PEP 565 has some discussion, and links to the earlier threads when DeprecationWarning was deemed too noisy for all the same reasons that this warning would be - it requires the library maintainer to fix it, not the end user.
As far as I’m aware, the only thing that’s changed since then is we probably have 10x more regular users who are simply going to learn to ignore warnings.
I haven’t looked it up, but I believe SyntaxWarning was added to warn about invalid escapes in string literals, which are far more likely to be caused by an interactive user. return in finally isn’t going to be written by an interactive user in any way that really matters - they’ll notice if it didn’t work, even if they don’t see an error (though life would be improved if they did see it).
So the issue is the warning is poorly targeted, and will be shown to the wrong people.
1 Like
Like assert with ().
I’d say that assert() is dramatically more likely to be written by an interactive user.
It also demonstrably has zero correctness (other than as a no-op), whereas try/finally/return is entirely valid and just has an unfortunate side-effect that may matter in exceptional cases.
I think it’s a huge stretch to argue that all uses of try/finally/return are broken enough to warn the users who didn’t write it and aren’t able to fix it. Appealing to existing SyntaxWarning cases isn’t going to help convince me (the other cases are calling literals and slicing literals with other literals instead of numbers, which can both be pretty easily caused by missing a comma).
2 Likes
This is where we disagree.
My analysis showed that most of the return-in-finally are incorrect.
The consequence is a swallowed exception, which is pretty bad and often hard to discover and debug. So I don’t see the harm as minor.
I do, on the other hand, consider the churn pretty small, given the overall number of instances is small (2-4 per million LOC).
Clearly 
Most is not all. And “incorrect” is not “broken” - by “incorrect” you mean “would swallow an exception”, which while annoying, does not affect the success case when no exception is thrown.
It’s easy for a maintainer to run a linter and discover the risk, and then make a fix.
I see the small number of instances as an argument in favour of us not needing to make such a big deal out of it. The amount of fixes can only be “pretty small” - it’s not worth permanently adding code into our compiler, impacting every single .py file that’s ever loaded for the rest of time, for the sake of 2-4 cases per million LOC.
It’s not worth training users that they should just ignore any printed output when they import <third-party module> in Python/Jupyter/etc, for the sake of 2-4 cases per million LOC.
It’s not worth breaking test suites that run with warnings-as-errors because one of their dependencies is slow to make a fix, for the sake of 2-4 cases per million LOC.
4 Likes
I’m with Steve on this one.
The compiler should not become a Python linter for things which may not be intended (but are valid Python code).
In fact, I think we should be working together with linter makers to move more such warnings, including deprecation warnings which can be detected at compile time to linters. Linters simply receive more visibility for these things by developers, while avoiding the negative impact on users.
5 Likes
That’s another point on which we disagree - I don’t see a swallowed exception as “annoying”, but as a really serious problem. That it only happens in an abnormal case (and is therefore likely to slip through testing) makes it worse.
2 Likes
Can’t we just hide syntax warnings outside of __main__ like DeprecationWarning?
# file.py
import warnings
def foo():
warnings.warn("msg", DeprecationWarning, stacklevel=2)
return "\p"
# file2.py
import file
file.foo()
# file3.py
import file2
$ python file.py
/tmp/file.py:6: SyntaxWarning: invalid escape sequence '\p'
$ python file2.py
/tmp/file.py:6: SyntaxWarning: invalid escape sequence '\p'
/tmp/file2.py:3: DeprecationWarning: msg
$ python file3.py
/tmp/file.py:6: SyntaxWarning: invalid escape sequence '\p'