PEP 765: Disallow return/break/continue that exit a finally block

We haven’t talked about.

While a PEP would be useful to lay out the counter arguments, opening an issue on the Steering Council GitHub gets it on our agenda. I recommend doing that asap if there’s anything that needs to be considered for 3.14. Please CC @hugovk on that issue for RM approval, which would also be needed for a 3.14 exception.

What Barry says. I will not be the one to open the issue though — I have had enough Python drama for several lifetimes.

16 Likes

Here’s a good post by @adamchainz that can be shared with upstream projects that trigger the warning:

3 Likes

This turned out more accurate than I could have ever guessed at the time. I didnt know that syntax warnings werent filtered properly, that uv doesn’t precompile by default, and that in spite of that and that these were decisions that were supposed to limit the disruptiveness, that when they did not function as intended that people would keep defending tossing a linter rule into the language without a plan to make it an error.

This is effectively more disruptive than traditional deprecation for the few affected projects.

CPython will emit a SyntaxWarning in version 3.14, and we leave it open whether, and when, this will become a SyntaxError

The PEP never committed to making it an error, which to me feels like this was just a way to do an end run around normal deprecation since you are now claiming that was always the intent for it to later be an error.

@hugovk the article there covers only the simplest case of try/finally. Nobody using it correctly has an issue with figuring out the right way to rewrite this, it’s the fact that working code has to be rewritten with no prior warning for what is amounts to a linter rule because it is impacting downstream code that is the problem. The warning breaks CI use of warning as errors immediately because this change was made without anyone verifying that warning filters would work (they don’t for this)

cases with nested try/finally while supressing errors require significantly more changes to properly retain the intened behavior, as outer finally blocks run even after returning in an inner one. This is a rather natural way to ensure ordered cleanup of things that must not fail to cleanup, and some libraries do intentionally suppress certain errors that their users wouldn’t be able to do anything about.

1 Like

I did not see any such cases in the empirical analysis I conducted of how this feature is used in real code.

8 Likes

I find that hard to believe. Both in general, and with the specific context of the discussion here trusting your anaylsis. I don’t have open source examples, but I know this is a pattern that exists in closed source code, and would be surprised that it doesn’t exist elsewhere.

It’s hardly the most important part of what I wrote as well.

You have argued with someone here involved with code in question who explained that it was intentional, and insisted on telling them it was actually a bug. You have revised history on it always being intended on becoming an error, and you went ahead with this change without actually verifying that doing it in a nonstandard way had the intended impact.

I’m actually in favor of removing return in finally from the language, I just have a lot of issues with how you’ve gone about it. Deprecation policies exist for a reason, and going around them and doing things in nonstandard ways while changing stances on if it even is meant to be an error isn’t helpful.

3 Likes

I said it reads like a bug. Not that it is actually a bug.

I said it should be an error, not that it was always intended to become an error.

Fortunately you don’t need to trust my analysis. My report included a detailed description on how I conducted it, along with the scripts I used, so that you can replicate it. If you think the methodology is flawed, you can explain why or suggest a better one.

12 Likes

This discussion feels a little unpleasant now.

Irit did some excellent research for this PEP and I’m sure also spent considerable time thinking about how to bring it forwards in the most helpful way possible for everyone else. You can see a discussion between Irit and myself before even the first pre-PEP discussion here. I believe that Irit had many similar discussions in other projects. This is far more research/effort than I have seen for anything similar to this and is absolutely to be commended.

The specific mechanism of a SyntaxWarning is awkward in some ways but I can certainly see the rationale. Emitting a warning at runtime is also awkward for something that potentially only happens on very unusual codepaths. The SyntaxWarning also sounds like a way of making sure that the right person sees the warning without others seeing it also.

This has caused problems for some people who run with warnings-as-errors. I have only limited sympathy for the people in that situation. It is perfectly reasonable to run with warnings-as-errors but if you do so then you can’t complain that you have errors because that is what you explicitly opted in to. The escape hatch is very clearly there: don’t turn warnings into errors (temporarily until resolved). Better filtering mechanisms are a reasonable request/suggestion but warnings-as-errors means you prefer errors so you can’t complain when you get them.

That being said I have just opened a pull request to change the offending line in SymPy. The code in question is not known to be used any more but removing it is much more likely to cause a problem for someone (and more headaches for me) than just changing it to avoid this SyntaxWarning. Making maintenance updates to code that is not used any more doesn’t feel like a productive use of my time though.

For libraries the warning has a knock-on impact because of dependents. For example PyTorch depends on SymPy and I would not want import torch to surface this warning. I have checked and I don’t believe it does because thankfully the import of the relevant module is delayed. I think that the set of people writing import torch contains many more less forgiving people (with pitchforks) than the set of people writing import sympy. this kind of thing can turn into a situation where we are pressured to make a release when it is not the right time to do so and not for any actually good reason.

I very much appreciate the effort Irit made to reach out which is exactly the right thing to do. Otherwise or following on from there though I would have preferred that this come via an update to ruff rather than a SyntaxWarning. Then a new version of ruff is released, CI fails, someone makes a PR, the problem is addressed and no one outside the project needs to know about it. There are many advantages doing it that way like the rule can be disabled or the code can be excluded from checks (a good option for code that is not used any more).

If the end goal is to remove/disable the feature then there does need to be some kind of warning in CPython itself. The PEP here does not guarantee removal but it seems clear that Irit wanted that, the research makes a good case, and I haven’t see any strong disagreements. It is still possible though to start with linters before warnings and maybe in future that approach makes more sense.

In any case I just want to make clear that while I am not sure if the SyntaxWarning is the best approach I do appreciate the excellent work that Irit did for this and I hope that we can see more of the same (from Irit and others).

28 Likes

I can’t say I particularly like the way in which this change was done either, but I’m also not going to argue to revert it. Removing return in finally is “fine”

Only thing I’d want to come out of this is for people adding new warnings to make sure warning filters actually work on the warnings they are adding, rather than just blaming people who use warnings as errors here.

The normal way of handling this (a warning filter) doesn’t work in this case. If it did, the obvious answer would be “if you’re fine with what the warning indicates, suppress it”. This is what people expect with warning as errors, and python has the tools for this, they just don’t work on this warning.

4 Likes

Thank you Oscar. More than a little. The personal tone was both unkind and unreasonable. The PEP has been accepted by the SC and if that was a mistake it’s a shared one.

I don’t like the SyntaxWarningeither, it was a compromise which doesn’t seem to have worked. I don’t know whether DeprecationWarningwith a target date for deprecation would have gone down better (or if anything could). If someone wants to propose that I will support it.

20 Likes

Personally I like the SyntaxWarning and to me it makes sense to have a SyntaxWarning and then never turn it into a hard error. I would just prefer that the process of updating code for things like this goes via linters because linter errors are only seen by the developers of the given codebase and come from tools that are designed to fit with a project’s development workflow.

There was a parallel thread about linters around the time of the discussions for this PEP which also has discussions about the limitations of the linter approach because of the myriad different lint rules and configurations. I think though that if there is a PEP such as this one that says “this will give a SyntaxWarning in Python 3.15” then that should be a good enough reason for linters to enable the rule by default (or even to emit warnings if the rule is disabled). The linter errors/warnings could clearly point to the PEP that says it will become a user-visible SyntaxWarning in Python itself. If that happened say a year before Python 3.15 is released then that is enough time for any maintained project to get an update out in its normal release cycle so that no one with latest Python and latest project foo ever sees the SyntaxWarning.

I think that SyntaxWarning is plenty enough to prevent any problems with this kind of thing without needing a SyntaxError. The reason I dislike a SyntaxError (or other hard error) is that it makes it difficult to do things like checkout old versions or bisect through older and newer versions of a Python codebase. You can use current Python to checkout SymPy from up to about 10 years ago and there are SyntaxWarnings but it still runs and can be tested. If you go any further back you just get a SyntaxError and it isn’t possible to test anything (I think you would need Python 2.7 which is becoming more difficult to obtain over time).

A couple of years ago I bisected a bug in Maxima which is computer algebra system written in Lisp and dating back to the late 60s. The git history there only goes back to somewhere around 2000 when the autotools build scripts were also added. I could bisect all the way over 25 years though using a stock Lisp compiler from the Ubuntu repos and ./configure && make && ./maxima mytest.mac. Plenty of warnings but the code always compiled and ran and I managed to bisect the offending commit.

I would like someone in 25 years time to be able to use whatever Python version they have to run through 35 years of the SymPy codebase. Warnings are fine but it only takes one SyntaxError to break that.

4 Likes

I wonder if there’s an argument for having a (small) set of linter-focused PEPs that standardise particular warnings that linters must provide? Similar to packaging interoperability standards, these would give an assurance that whatever linting tool someone used, they could check for issues like this one that the core team have decided need to be reported to the developer. This would then be possible without having to build the warning into the runtime, where as we’ve seen it’s hard to avoid the wrong people getting the message.

5 Likes

I’m not sure it’s within the SC remit to determine what linters must do. That means this proposal would not solve the problem, maybe make it worse by taking part of the deprecation process out of the core devs’ hands. If one linter doesn’t include the warning we can’t proceed, because someone who relies on it runs their CI with -We and we’re back to where we started.

You could say the same for packaging standards, but the packaging community has organised around PEPs for their interoperability specifications. I don’t know if there’s enough of a “linter community”[1] to do something similar.

But my point was that if we’re writing a PEP that says “such-and-such should be a syntax warning” and we find that we want something less invasive than an actual core interpreter syntax warning, it’s not that much of a stretch to change the PEP to say “linters should (or even must) report this issue if they see it”. But only if there’s a sufficiently strong community around linter rules to support such a statement (that’s the bit the SC can’t control).


  1. Isn’t there a PyCQA - Python Code Quality Authority? Or is that simply humourous like the PyPA was originally intended to be? ↩︎

There is: https://meta.pycqa.org

It is. From the code-quality mailing list in 2014

It’s in the tradition of PyCA and PyPA (Python Cryptographic and Packaging
Authorities), but it can be renamed before we add any repos to it since it can sound rather presumptuous to some (even though it’s mostly in jest).

1 Like

Do they have any special authority? Compared to, e.g., the wonder pylint or Ruff teams.

No:

“PyCQA” is an abbreviated name for “Python Code Quality Authority”.

The PyCQA is not actually an authority on anything. (See also Creation of the PyCQA.)

What is the PyCQA?

The PyCQA is a loose organization of people who maintain projects in roughly the same domain: automatic style and quality reporting. Almost all of these projects are widely used by the larger Python community (and by each other) to enforce style guidelines and maintain some modicum of consistency within a code base. The organization formed around these people to provide them an easy to remember namespace for their projects.

I just collected all SyntaxWarnings in the CPython source code and reviewed whether Ruff catches them:

For reference here is a code block that triggers all of them:

x = [x + 1for x in [1]]  # SyntaxWarning: invalid decimal literal (ruff: nothing)
x = "\99"  # SyntaxWarning: "\9" is an invalid escape sequence (ruff: W605)
x = f"\{1}"  # SyntaxWarning: "\{" is an invalid escape sequence (ruff: W605)


def _f() -> int:
    try:
        return 1 / 0
    finally:
        return 42  # SyntaxWarning: 'return' in a 'finally' block (ruff: B012)


x = () is ()  # SyntaxWarning: "is" with 'tuple' literal (ruff: F632)
assert (1,)  # SyntaxWarning: assertion is always true, perhaps remove parentheses? (ruff: F631)
{}()  # SyntaxWarning: 'dict' object is not callable (ruff: nothing)
{1}[1]  # SyntaxWarning: 'set' object is not subscriptable (ruff: nothing)
"a"["b"]  # SyntaxWarning: str indices must be integers or slices, not str (ruff: RUF016)

Most cases already trigger Ruff warnings, though a few are missing. I also tried the two other major linters, pylint and flake8, but didn’t write things up in as much detail. Pylint also catches most of these issues. Flake8 (more precisely, pycodestyle) crashes on this sample, but should also catch many of these issues; after all, most Ruff rules derive from flake8 and its plugins.

The PEP 765 SyntaxWarning is caught by all three linters (Ruff B012; pylint W0134 and W0150; flake8 B012 and SIM107, both from plugins).

So I don’t think the suggestion in this thread that there should be some mandate on linters to catch this issue is a good idea: linters already catch this.

13 Likes

Only if the rule is enabled. Do they enable it by default?

How much time do you want to spend selecting lint rules for a large existing codebase from the ruff list?

Would you enable the whole of the B or SIM categories? Why those categories and have you evaluated every other possible category?

Either of these categories might throw up thousands of false positives because of other pointless rules like these ones:

If you enable just these two rules in the sympy codebase you get:

$ ruff check sympy --statistics
910	SIM300	[*] yoda-conditions
243	B007  	[ ] unused-loop-control-variable
Found 1153 errors.
[*] 910 fixable with the `--fix` option (165 hidden fixes can be enabled with the `--unsafe-fixes` option).

An example of each case is:

sympy/utilities/tests/test_wester.py:3008:12: SIM300 [*] Yoda condition detected
     |
3006 |
3007 |     F, _, _ =  mellin_transform(1/(1 - x), x, s)
3008 |     assert F == pi*cot(pi*s)
     |            ^^^^^^^^^^^^^^^^^ SIM300
     |
     = help: Rewrite as `pi*cot(pi*s) == F`

sympy/utilities/_compilation/util.py:174:15: B007 Loop control variable `dirs` not used within loop body
    |
172 |         cwd = '.'
173 |     globbed = []
174 |     for root, dirs, filenames in os.walk(cwd):
    |               ^^^^ B007
175 |         for fn in filenames:
    |
    = help: Rename unused `dirs` to `_dirs`

Does either of those look like a meaningful improvement? Do you want to review 1000 examples of these? In either case would you want to impose this kind of rule on all future contributors?

How much time do you want to spend making pointless changes in the code so that you can use the SIM category or otherwise carefully analysing what each of the different rules in the category is doing to select which ones to include/exclude?

Driveby contributors often open pull requests to sympy attempting to apply seemingly random choices from among these rules and fixers. I can’t think of a time when any of them has actually picked a genuinely useful rule though so those kinds of PRs are just a drain. Do you want to review a 1000 line diff touching every part of a large codebase from some new contributor that you have no prior trust in and where the change provides zero meaningful improvement?

This is why sympy did not have either the B012 or SIM107 rules enabled in either flake8 or ruff even though both linters are run in CI. If these rules are important then they should not be buried in a mess of random opinionated stuff.

I don’t see why special authority is really needed though. The current way this generally works is:

  • Someone makes a flake8 plugin with some rules
  • Other people copy the rules into ruff

Anyone can do either of these things but thus far the result of people doing this is the sets of rules that you can see for ruff here. The problem is that there is no plugin/section containing rules that are non-opinionated and universally applicable with low false positive rate. I think if someone carefully curated such a list of rules then the situation could arise where it could be considered if linters should apply those rules by default.

I think the problem is that people who make these rule sets are typically the kind of people who like to have lots of opinionated rules. Each rule set is really just the author’s personal rule fiefdom rather than being any meaningful categorisation from the perspective of anyone else choosing which rules to use.

There is nothing stopping anyone from making more conservative rule sets though and I imagine linter maintainers would welcome that as well.

3 Likes

… but not with the default configuration, as stated in the documentation:

By default, Ruff enables Flake8’s F rules, along with a subset of the E rules, omitting any stylistic rules that overlap with the use of a formatter, like ruff format or Black.

Maybe there could be some incentive so that the linters catch the SyntaxWarning by default ? Given the huge list of rules (see link above), I’m sure that many people don’t look too much into which ones to enable.

1 Like