From b2f81269764f1efab4fd4778dd297436a29ebf29 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 18 Nov 2024 23:14:48 +0000 Subject: [PATCH 1/8] PEP 765: add report as appendix --- peps/pep-0765.rst | 233 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 232 insertions(+), 1 deletion(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index f2036b0b1a8..2531172fd19 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -54,7 +54,7 @@ Rationale ========= A recent `analysis of real world code -`_ shows that: +`__ shows that: - These features are rare (2 per million LOC in the top 8,000 PyPI packages, 4 per million LOC in a random selection of packages). @@ -64,6 +64,8 @@ A recent `analysis of real world code - Code owners are typically receptive to fixing the bugs, and find that easy to do. +See `the appendix<#appendix>`__ for more details. + This new data indicates that it would benefit Python's users if Python itself moved them away from this harmful feature. @@ -154,6 +156,235 @@ that their code needs to change. The `empirical evidence shows that the changes necessary are typically quite straightforward. +Appendix +======== + +Below is an abridged version of a +`research report`__ +by Irit Katriel, which was posted on 9 Nov 2024. + +``return`` in ``finally`` considered harmful +-------------------------------------------- + +I describe an investigation of usage of ``return``, ``break`` and ``continue`` +in a ``finally`` clause in real world code. It addresses the +questions: Are people using it? How often are they using it incorrectly? +How much churn would the proposed change create? + +Method +^^^^^^ + +The analysis is based on the 8,000 most popular PyPI packages, in terms of number +of downloads in the last 30 days. They were downloaded on the 17th-18th of +October, using +`a script`__ +written by Guido van Rossum, which in turn relies on Hugo van Kemenade's +`tool`__ that creates a list of the +most popular packages. + +Once downloaded, a +`second script`__ +was used to construct an AST for each file, and traverse it to identify ``break``, +``continue`` and ``return`` statements which are directly inside a ``finally`` block. + +I then found the current source code for each occurence, and categorized it. For +cases where the code seems incorrect, I created an issue in the project's bug +tracker. The responses to these issues are also part of the data collected in +this investigation. + +Results +^^^^^^^ + +I decided not to include a list of the incorrect usages, out of concern that +it would make this report look like a shaming exercise. Instead I will describe +the results in general terms, but will mention that some of the problems I found +appear in very popular libraries, including a cloud security application. +For those so inclined, it should not be hard to replicate my analysis, as I +provided links to the scripts I used in the Method section. + +The projects examined contained a total of 120,964,221 lines of Python code, +and among them the script found 203 instances of control flow instructions in a +``finally`` block. Most were ``return``, a handful were ``break``, and none were +``continue``. Of these: + +- 46 are correct, and appear in tests that target this pattern as a feature (e.g., +tests for linters that detect it). +- 8 seem like they could be correct - either intentionally swallowing exceptions +or appearing where an active exception cannot occur. Despite being correct, it is +not hard to rewrite them to avoid the bad pattern, and it would make the code +clearer: deliberately swallowing exceptions can be more explicitly done with +``except BaseException:``, and ``return`` which doesn't swallow exceptions can be +moved after the ``finally`` block. +- 149 were clearly incorrect, and can lead to unintended swallowing of exceptions. +These are analyzed in the next section. + +**The Error Cases** + +Many of the error cases followed this pattern: + +.. code-block:: + :class: bad + + try: + ... + except SomeSpecificError: + ... + except Exception: + logger.log(...) + finally: + return some_value + +Code like this is obviously incorrect because it deliberately logs and swallows +``Exception`` subclasses, while silently swallowing ``BaseExceptions``. The intention +here is either to allow ``BaseExceptions`` to propagate on, or (if the author is +unaware of the ``BaseException`` issue), to log and swallow all exceptions. However, +even if the ``except Exception`` was changed to ``except BaseException``, this code +would still have the problem that the ``finally`` block swallows all exceptions +raised from within the ``except`` block, and this is probably not the intention +(if it is, that can be made explicit with another ``try``-``except BaseException``). + +Another variation on the issue found in real code looks like this: + +.. code-block:: + :class: bad + + try: + ... + except: + return NotImplemented + finally: + return some_value + +Here the intention seems to be to return ``NotImplemented`` when an exception is +raised, but the ``return`` in the ``finally`` block would override the one in the +``except`` block. + +> [!NOTE] +> Following the +> `discussion`__, +> I repeated the analysis on a random selection of PyPI packages (to +> analyze code written by *average* programmers). The sample contained +> in total 77,398,892 lines of code with 316 instances of ``return``/``break``/``continue`` +> in ``finally``. So about 4 instances per million lines of code. + +**Author reactions** + +Of the 149 incorrect instances of ``return`` or ``break`` in a ``finally`` clause, +27 were out of date, in the sense that they do not appear in the main/master branch +of the library, as the code has been deleted or fixed by now. The remaining 122 +are in 73 different packages, and I created an issue in each one to alert the +authors to the problems. Within two weeks, 40 of the 73 issues received a reaction +from the code maintainers: + +- 15 issues had a PR opened to fix the problem. +- 20 received reactions acknowleding the problem as one worth looking into. +- 3 replied that the code is no longer maintained so this won't be fixed. +- 2 closed the issue as "works as intended", one said that they intend to +swallow all exceptions, but the other seemed unaware of the distinction +between ``Exception`` and ``BaseException``. + +One issue was linked to a pre-existing open issue about non-responsiveness to Ctrl-C, +conjecturing a connection. + +Two of the issue were labelled as "good first issue". + +**The correct usages** + +The 8 cases where the feature appears to be used correctly (in non-test code) also +deserve attention. These represent the "churn" that would be caused by blocking +the feature, because this is where working code will need to change. I did not +contact the authors in these cases, so we will need to assess the difficulty of +making these changes ourselves. + +- In `mosaicml`__ +there is a return in a finally at the end of the ``main`` function, after an ``except:`` +clause which swallows all exceptions. The return in the finally would swallow +an exception raised from within the ``except:`` clause, but this seems to be the +intention in this code. A possible fix would be to assign the return value to a +variable in the ``finally`` clause, dedent the ``return`` statement and wrap the +body of the ``except:`` clause by another ``try``-``except`` to swallow any +exceptions from within it. + +- In `webtest`__ +there is a ``finally`` block that contains only ``return False``. It could be replaced +by + +.. code-block:: + + except BaseException: + pass + return False + +- In `kivy`__ +there is a ``finally`` that contains only a ``return`` statement. Since there is also a +bare ``except`` just before it, in this case the fix will be to just remove the ``finally:`` +block and dedent the ``return`` statement. + +- In `logilab-common`__ +there is, once again, a ``finally`` clause that can be replace by an ``except BaseException`` +with the ``return`` dedented one level. + +- In `pluggy`__ +there is a lengthy ``finally`` with two ``return`` statements (the second on line 182). Here the +return value can be assigned to a variable, and the ``return`` itself can appear after we've +exited the ``finally`` clause. + +- In `aws-sam-cli`__ +there is a conditional return at the end of the block. +From reading the code, it seems that the condition only holds when the exception has +been handled. The conditional block can just move outside of the ``finally`` block and +achieve the same effect. + +- In `scrappy`__ +there is a ``finally`` that contains only a ``break`` instruction. Assuming that it was the intention +to swallow all exceptions, it can be replaced by + +.. code-block:: + + except BaseException: + pass + break + +Discussion +^^^^^^^^^^ + +The first thing to note is that ``return``/``break``/``continue`` in a ``finally`` +block is not something we see often: 203 instance in over 120 million lines +of code. This is, possibly, thanks to the linters that warn about this. + +The second observation is that most of the usages were incorrect: 73% in our +sample (149 of 203). + +Finally, the author responses were overwhelmingly positive. Of the 40 responses +received within two weeks, 35 acknowledged the issue, 15 of which also created +a PR to fix it. Only two thought that the code is fine as it is, and three +stated that the code is no longer maintained so they will not look into it. + +The 8 instances where the code seems to work as intended, are not hard to +rewrite. + +Conclusion +^^^^^^^^^^ + +The results indicate that ``return``, ``break`` and ``continue`` in a finally block + +- are rarely used. +- when they are used, they are usually used incorrectly. +- code authors are receptive to changing their code, and tend to find it easy to do. + +## Acknowledgements + +I thank: + +- Alyssa Coghlan for +`bringing this issue my attention`__. + +- Guido van Rossum and Hugo van Kemenade for the +`script`__ +that downloads the most popular PyPI packages. + +- The many code authors I contacted for their responsiveness and grace. + Copyright ========= From 95f8744ea0ad3265f1c394bf160b02a952a6e073 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 18 Nov 2024 23:27:42 +0000 Subject: [PATCH 2/8] whitespace --- peps/pep-0765.rst | 70 +++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index 2531172fd19..21adfa90723 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -208,15 +208,15 @@ and among them the script found 203 instances of control flow instructions in a ``continue``. Of these: - 46 are correct, and appear in tests that target this pattern as a feature (e.g., -tests for linters that detect it). + tests for linters that detect it). - 8 seem like they could be correct - either intentionally swallowing exceptions -or appearing where an active exception cannot occur. Despite being correct, it is -not hard to rewrite them to avoid the bad pattern, and it would make the code -clearer: deliberately swallowing exceptions can be more explicitly done with -``except BaseException:``, and ``return`` which doesn't swallow exceptions can be -moved after the ``finally`` block. + or appearing where an active exception cannot occur. Despite being correct, it is + not hard to rewrite them to avoid the bad pattern, and it would make the code + clearer: deliberately swallowing exceptions can be more explicitly done with + ``except BaseException:``, and ``return`` which doesn't swallow exceptions can be + moved after the ``finally`` block. - 149 were clearly incorrect, and can lead to unintended swallowing of exceptions. -These are analyzed in the next section. + These are analyzed in the next section. **The Error Cases** @@ -280,8 +280,8 @@ from the code maintainers: - 20 received reactions acknowleding the problem as one worth looking into. - 3 replied that the code is no longer maintained so this won't be fixed. - 2 closed the issue as "works as intended", one said that they intend to -swallow all exceptions, but the other seemed unaware of the distinction -between ``Exception`` and ``BaseException``. + swallow all exceptions, but the other seemed unaware of the distinction + between ``Exception`` and ``BaseException``. One issue was linked to a pre-existing open issue about non-responsiveness to Ctrl-C, conjecturing a connection. @@ -297,17 +297,17 @@ contact the authors in these cases, so we will need to assess the difficulty of making these changes ourselves. - In `mosaicml`__ -there is a return in a finally at the end of the ``main`` function, after an ``except:`` -clause which swallows all exceptions. The return in the finally would swallow -an exception raised from within the ``except:`` clause, but this seems to be the -intention in this code. A possible fix would be to assign the return value to a -variable in the ``finally`` clause, dedent the ``return`` statement and wrap the -body of the ``except:`` clause by another ``try``-``except`` to swallow any -exceptions from within it. + there is a return in a finally at the end of the ``main`` function, after an ``except:`` + clause which swallows all exceptions. The return in the finally would swallow + an exception raised from within the ``except:`` clause, but this seems to be the + intention in this code. A possible fix would be to assign the return value to a + variable in the ``finally`` clause, dedent the ``return`` statement and wrap the + body of the ``except:`` clause by another ``try``-``except`` to swallow any + exceptions from within it. - In `webtest`__ -there is a ``finally`` block that contains only ``return False``. It could be replaced -by + there is a ``finally`` block that contains only ``return False``. It could be replaced + by .. code-block:: @@ -316,28 +316,28 @@ by return False - In `kivy`__ -there is a ``finally`` that contains only a ``return`` statement. Since there is also a -bare ``except`` just before it, in this case the fix will be to just remove the ``finally:`` -block and dedent the ``return`` statement. + there is a ``finally`` that contains only a ``return`` statement. Since there is also a + bare ``except`` just before it, in this case the fix will be to just remove the ``finally:`` + block and dedent the ``return`` statement. - In `logilab-common`__ -there is, once again, a ``finally`` clause that can be replace by an ``except BaseException`` -with the ``return`` dedented one level. + there is, once again, a ``finally`` clause that can be replace by an ``except BaseException`` + with the ``return`` dedented one level. - In `pluggy`__ -there is a lengthy ``finally`` with two ``return`` statements (the second on line 182). Here the -return value can be assigned to a variable, and the ``return`` itself can appear after we've -exited the ``finally`` clause. + there is a lengthy ``finally`` with two ``return`` statements (the second on line 182). Here the + return value can be assigned to a variable, and the ``return`` itself can appear after we've + exited the ``finally`` clause. - In `aws-sam-cli`__ -there is a conditional return at the end of the block. -From reading the code, it seems that the condition only holds when the exception has -been handled. The conditional block can just move outside of the ``finally`` block and -achieve the same effect. + there is a conditional return at the end of the block. + From reading the code, it seems that the condition only holds when the exception has + been handled. The conditional block can just move outside of the ``finally`` block and + achieve the same effect. - In `scrappy`__ -there is a ``finally`` that contains only a ``break`` instruction. Assuming that it was the intention -to swallow all exceptions, it can be replaced by + there is a ``finally`` that contains only a ``break`` instruction. Assuming that it was the intention + to swallow all exceptions, it can be replaced by .. code-block:: @@ -377,11 +377,11 @@ The results indicate that ``return``, ``break`` and ``continue`` in a finally bl I thank: - Alyssa Coghlan for -`bringing this issue my attention`__. + `bringing this issue my attention`__. - Guido van Rossum and Hugo van Kemenade for the -`script`__ -that downloads the most popular PyPI packages. + `script`__ + that downloads the most popular PyPI packages. - The many code authors I contacted for their responsiveness and grace. From c515aa4ae8c33eb92ffeecbecc53ab2c7b42ca3e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 18 Nov 2024 23:34:59 +0000 Subject: [PATCH 3/8] whitespace --- peps/pep-0765.rst | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index 21adfa90723..ba28852b891 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -160,7 +160,7 @@ Appendix ======== Below is an abridged version of a -`research report`__ +`research report `__ by Irit Katriel, which was posted on 9 Nov 2024. ``return`` in ``finally`` considered harmful @@ -177,13 +177,13 @@ Method The analysis is based on the 8,000 most popular PyPI packages, in terms of number of downloads in the last 30 days. They were downloaded on the 17th-18th of October, using -`a script`__ +`a script `__ written by Guido van Rossum, which in turn relies on Hugo van Kemenade's -`tool`__ that creates a list of the +`tool `__ that creates a list of the most popular packages. Once downloaded, a -`second script`__ +`second script `__ was used to construct an AST for each file, and traverse it to identify ``break``, ``continue`` and ``return`` statements which are directly inside a ``finally`` block. @@ -261,7 +261,7 @@ raised, but the ``return`` in the ``finally`` block would override the one in th > [!NOTE] > Following the -> `discussion`__, +> `discussion `__, > I repeated the analysis on a random selection of PyPI packages (to > analyze code written by *average* programmers). The sample contained > in total 77,398,892 lines of code with 316 instances of ``return``/``break``/``continue`` @@ -296,7 +296,7 @@ the feature, because this is where working code will need to change. I did not contact the authors in these cases, so we will need to assess the difficulty of making these changes ourselves. -- In `mosaicml`__ +- In `mosaicml `__ there is a return in a finally at the end of the ``main`` function, after an ``except:`` clause which swallows all exceptions. The return in the finally would swallow an exception raised from within the ``except:`` clause, but this seems to be the @@ -305,7 +305,7 @@ making these changes ourselves. body of the ``except:`` clause by another ``try``-``except`` to swallow any exceptions from within it. -- In `webtest`__ +- In `webtest `__ there is a ``finally`` block that contains only ``return False``. It could be replaced by @@ -315,27 +315,27 @@ making these changes ourselves. pass return False -- In `kivy`__ +- In `kivy `__ there is a ``finally`` that contains only a ``return`` statement. Since there is also a bare ``except`` just before it, in this case the fix will be to just remove the ``finally:`` block and dedent the ``return`` statement. -- In `logilab-common`__ +- In `logilab-common `__ there is, once again, a ``finally`` clause that can be replace by an ``except BaseException`` with the ``return`` dedented one level. -- In `pluggy`__ +- In `pluggy `__ there is a lengthy ``finally`` with two ``return`` statements (the second on line 182). Here the return value can be assigned to a variable, and the ``return`` itself can appear after we've exited the ``finally`` clause. -- In `aws-sam-cli`__ +- In `aws-sam-cli `__ there is a conditional return at the end of the block. From reading the code, it seems that the condition only holds when the exception has been handled. The conditional block can just move outside of the ``finally`` block and achieve the same effect. -- In `scrappy`__ +- In `scrappy `__ there is a ``finally`` that contains only a ``break`` instruction. Assuming that it was the intention to swallow all exceptions, it can be replaced by @@ -377,10 +377,10 @@ The results indicate that ``return``, ``break`` and ``continue`` in a finally bl I thank: - Alyssa Coghlan for - `bringing this issue my attention`__. + `bringing this issue my attention `__. - Guido van Rossum and Hugo van Kemenade for the - `script`__ + `script `__ that downloads the most popular PyPI packages. - The many code authors I contacted for their responsiveness and grace. From 42f3474bb384a3daa4cfdae46ff65b1ff659a6e5 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 19 Nov 2024 00:06:45 +0000 Subject: [PATCH 4/8] fixit --- peps/pep-0765.rst | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index ba28852b891..a7c532d5024 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -53,8 +53,8 @@ flag them as a problem. Rationale ========= -A recent `analysis of real world code -`__ shows that: +A recent +`analysis of real world code `__ shows that: - These features are rare (2 per million LOC in the top 8,000 PyPI packages, 4 per million LOC in a random selection of packages). @@ -64,13 +64,13 @@ A recent `analysis of real world code - Code owners are typically receptive to fixing the bugs, and find that easy to do. -See `the appendix<#appendix>`__ for more details. +See `the appendix <#appendix>`__ for more details. This new data indicates that it would benefit Python's users if Python itself moved them away from this harmful feature. -One of the arguments brought up in `the PEP 601 discussion -`__ +One of the arguments brought up in +`the PEP 601 discussion `__ was that language features should be orthogonal, and combine without context-based restrictions. However, in the meantime :pep:`654` has been implemented, and it forbids ``return``, ``break`` and ``continue`` @@ -151,8 +151,7 @@ How to Teach This The change will be documented in the language spec and in the What's New documentation. The ``SyntaxWarning`` will alert users -that their code needs to change. The `empirical evidence -`__ +that their code needs to change. The `empirical evidence <#appendix>`__ shows that the changes necessary are typically quite straightforward. From db58d0dc7f4a932b7e566d4953ed2fe7adb52672 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 19 Nov 2024 12:33:06 +0000 Subject: [PATCH 5/8] abridge some more --- peps/pep-0765.rst | 88 +++++------------------------------------------ 1 file changed, 9 insertions(+), 79 deletions(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index a7c532d5024..0ffbde87c84 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -158,15 +158,14 @@ straightforward. Appendix ======== -Below is an abridged version of a -`research report `__ -by Irit Katriel, which was posted on 9 Nov 2024. - ``return`` in ``finally`` considered harmful -------------------------------------------- -I describe an investigation of usage of ``return``, ``break`` and ``continue`` -in a ``finally`` clause in real world code. It addresses the +Below is an abridged version of a +`research report `__ +by Irit Katriel, which was posted on 9 Nov 2024. +It describes an investigation into usage of ``return``, ``break`` and ``continue`` +in a ``finally`` clause in real world code, addressing the questions: Are people using it? How often are they using it incorrectly? How much churn would the proposed change create? @@ -292,57 +291,10 @@ Two of the issue were labelled as "good first issue". The 8 cases where the feature appears to be used correctly (in non-test code) also deserve attention. These represent the "churn" that would be caused by blocking the feature, because this is where working code will need to change. I did not -contact the authors in these cases, so we will need to assess the difficulty of -making these changes ourselves. - -- In `mosaicml `__ - there is a return in a finally at the end of the ``main`` function, after an ``except:`` - clause which swallows all exceptions. The return in the finally would swallow - an exception raised from within the ``except:`` clause, but this seems to be the - intention in this code. A possible fix would be to assign the return value to a - variable in the ``finally`` clause, dedent the ``return`` statement and wrap the - body of the ``except:`` clause by another ``try``-``except`` to swallow any - exceptions from within it. - -- In `webtest `__ - there is a ``finally`` block that contains only ``return False``. It could be replaced - by - -.. code-block:: - - except BaseException: - pass - return False - -- In `kivy `__ - there is a ``finally`` that contains only a ``return`` statement. Since there is also a - bare ``except`` just before it, in this case the fix will be to just remove the ``finally:`` - block and dedent the ``return`` statement. - -- In `logilab-common `__ - there is, once again, a ``finally`` clause that can be replace by an ``except BaseException`` - with the ``return`` dedented one level. - -- In `pluggy `__ - there is a lengthy ``finally`` with two ``return`` statements (the second on line 182). Here the - return value can be assigned to a variable, and the ``return`` itself can appear after we've - exited the ``finally`` clause. - -- In `aws-sam-cli `__ - there is a conditional return at the end of the block. - From reading the code, it seems that the condition only holds when the exception has - been handled. The conditional block can just move outside of the ``finally`` block and - achieve the same effect. - -- In `scrappy `__ - there is a ``finally`` that contains only a ``break`` instruction. Assuming that it was the intention - to swallow all exceptions, it can be replaced by - -.. code-block:: - - except BaseException: - pass - break +contact the authors in these cases, so we need to assess the difficulty of +making these changes ourselves. It is show in +`the full report`__, +that the change required in each case is small. Discussion ^^^^^^^^^^ @@ -362,28 +314,6 @@ stated that the code is no longer maintained so they will not look into it. The 8 instances where the code seems to work as intended, are not hard to rewrite. -Conclusion -^^^^^^^^^^ - -The results indicate that ``return``, ``break`` and ``continue`` in a finally block - -- are rarely used. -- when they are used, they are usually used incorrectly. -- code authors are receptive to changing their code, and tend to find it easy to do. - -## Acknowledgements - -I thank: - -- Alyssa Coghlan for - `bringing this issue my attention `__. - -- Guido van Rossum and Hugo van Kemenade for the - `script `__ - that downloads the most popular PyPI packages. - -- The many code authors I contacted for their responsiveness and grace. - Copyright ========= From 50435ef472ba32c328d3d6da4fb37e27e6265e41 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Tue, 19 Nov 2024 13:11:38 +0000 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- peps/pep-0765.rst | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index 0ffbde87c84..f1e9463843a 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -185,7 +185,7 @@ Once downloaded, a was used to construct an AST for each file, and traverse it to identify ``break``, ``continue`` and ``return`` statements which are directly inside a ``finally`` block. -I then found the current source code for each occurence, and categorized it. For +I then found the current source code for each occurrence, and categorized it. For cases where the code seems incorrect, I created an issue in the project's bug tracker. The responses to these issues are also part of the data collected in this investigation. @@ -257,13 +257,12 @@ Here the intention seems to be to return ``NotImplemented`` when an exception is raised, but the ``return`` in the ``finally`` block would override the one in the ``except`` block. -> [!NOTE] -> Following the -> `discussion `__, -> I repeated the analysis on a random selection of PyPI packages (to -> analyze code written by *average* programmers). The sample contained -> in total 77,398,892 lines of code with 316 instances of ``return``/``break``/``continue`` -> in ``finally``. So about 4 instances per million lines of code. +.. note:: Following the + `discussion `__, + I repeated the analysis on a random selection of PyPI packages (to + analyze code written by *average* programmers). The sample contained + in total 77,398,892 lines of code with 316 instances of ``return``/``break``/``continue`` + in ``finally``. So about 4 instances per million lines of code. **Author reactions** @@ -275,7 +274,7 @@ authors to the problems. Within two weeks, 40 of the 73 issues received a reacti from the code maintainers: - 15 issues had a PR opened to fix the problem. -- 20 received reactions acknowleding the problem as one worth looking into. +- 20 received reactions acknowledging the problem as one worth looking into. - 3 replied that the code is no longer maintained so this won't be fixed. - 2 closed the issue as "works as intended", one said that they intend to swallow all exceptions, but the other seemed unaware of the distinction From cd7c5cf134774af0452907330eb386440256b1ae Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Tue, 19 Nov 2024 13:22:23 +0000 Subject: [PATCH 7/8] Update peps/pep-0765.rst Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- peps/pep-0765.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index f1e9463843a..a5b066b22d8 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -292,7 +292,7 @@ deserve attention. These represent the "churn" that would be caused by blocking the feature, because this is where working code will need to change. I did not contact the authors in these cases, so we need to assess the difficulty of making these changes ourselves. It is show in -`the full report`__, +`the full report `__, that the change required in each case is small. Discussion From 48ea9855a9684edecc5e80c88674adabef13fb4e Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Tue, 19 Nov 2024 13:39:41 +0000 Subject: [PATCH 8/8] Update peps/pep-0765.rst Co-authored-by: Alex Waygood --- peps/pep-0765.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index a5b066b22d8..ffaaca31174 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -291,7 +291,7 @@ The 8 cases where the feature appears to be used correctly (in non-test code) al deserve attention. These represent the "churn" that would be caused by blocking the feature, because this is where working code will need to change. I did not contact the authors in these cases, so we need to assess the difficulty of -making these changes ourselves. It is show in +making these changes ourselves. It is shown in `the full report `__, that the change required in each case is small.