@@ -158,15 +158,14 @@ straightforward.
158158Appendix
159159========
160160
161- Below is an abridged version of a
162- `research report <https://github.com/iritkatriel/finally/commits/main/README.md >`__
163- by Irit Katriel, which was posted on 9 Nov 2024.
164-
165161``return `` in ``finally `` considered harmful
166162--------------------------------------------
167163
168- I describe an investigation of usage of ``return ``, ``break `` and ``continue ``
169- in a ``finally `` clause in real world code. It addresses the
164+ Below is an abridged version of a
165+ `research report <https://github.com/iritkatriel/finally/commits/main/README.md >`__
166+ by Irit Katriel, which was posted on 9 Nov 2024.
167+ It describes an investigation into usage of ``return ``, ``break `` and ``continue ``
168+ in a ``finally `` clause in real world code, addressing the
170169questions: Are people using it? How often are they using it incorrectly?
171170How much churn would the proposed change create?
172171
@@ -292,57 +291,10 @@ Two of the issue were labelled as "good first issue".
292291The 8 cases where the feature appears to be used correctly (in non-test code) also
293292deserve attention. These represent the "churn" that would be caused by blocking
294293the feature, because this is where working code will need to change. I did not
295- contact the authors in these cases, so we will need to assess the difficulty of
296- making these changes ourselves.
297-
298- - In `mosaicml <https://github.com/mosaicml/composer/blob/694e72159cf026b838ba00333ddf413185b4fb4f/composer/cli/launcher.py#L590 >`__
299- there is a return in a finally at the end of the ``main `` function, after an ``except: ``
300- clause which swallows all exceptions. The return in the finally would swallow
301- an exception raised from within the ``except: `` clause, but this seems to be the
302- intention in this code. A possible fix would be to assign the return value to a
303- variable in the ``finally `` clause, dedent the ``return `` statement and wrap the
304- body of the ``except: `` clause by another ``try ``-``except `` to swallow any
305- exceptions from within it.
306-
307- - In `webtest <https://github.com/Pylons/webtest/blob/617a2b823c60e8d7c5f6e12a220affbc72e09d7d/webtest/http.py#L131 >`__
308- there is a ``finally `` block that contains only ``return False ``. It could be replaced
309- by
310-
311- .. code-block ::
312-
313- except BaseException:
314- pass
315- return False
316-
317- - In `kivy <https://github.com/kivy/kivy/blob/3b744c7ed274c1a99bd013fc55a5191ebd8a6a40/kivy/uix/codeinput.py#L204 >`__
318- there is a ``finally `` that contains only a ``return `` statement. Since there is also a
319- bare ``except `` just before it, in this case the fix will be to just remove the ``finally: ``
320- block and dedent the ``return `` statement.
321-
322- - In `logilab-common <https://forge.extranet.logilab.fr/open-source/logilab-common/-/blob/branch/default/test/data/module.py?ref_type=heads#L60 >`__
323- there is, once again, a ``finally `` clause that can be replace by an ``except BaseException ``
324- with the ``return `` dedented one level.
325-
326- - In `pluggy <https://github.com/pytest-dev/pluggy/blob/c760a77e17d512c3572d54d368fe6c6f9a7ac810/src/pluggy/_callers.py#L141 >`__
327- there is a lengthy ``finally `` with two ``return `` statements (the second on line 182). Here the
328- return value can be assigned to a variable, and the ``return `` itself can appear after we've
329- exited the ``finally `` clause.
330-
331- - In `aws-sam-cli <https://github.com/aws/aws-sam-cli/blob/97e63dcc2738529eded8eecfef4b875abc3a476f/samcli/local/apigw/local_apigw_service.py#L721 >`__
332- there is a conditional return at the end of the block.
333- From reading the code, it seems that the condition only holds when the exception has
334- been handled. The conditional block can just move outside of the ``finally `` block and
335- achieve the same effect.
336-
337- - In `scrappy <https://github.com/scrapy/scrapy/blob/52c072640aa61884de05214cb1bdda07c2a87bef/scrapy/utils/gz.py#L27 >`__
338- there is a ``finally `` that contains only a ``break `` instruction. Assuming that it was the intention
339- to swallow all exceptions, it can be replaced by
340-
341- .. code-block ::
342-
343- except BaseException:
344- pass
345- break
294+ contact the authors in these cases, so we need to assess the difficulty of
295+ making these changes ourselves. It is show in
296+ `the full report<https://github.com/iritkatriel/finally/commits/main/README.md> `__,
297+ that the change required in each case is small.
346298
347299Discussion
348300^^^^^^^^^^
@@ -362,28 +314,6 @@ stated that the code is no longer maintained so they will not look into it.
362314The 8 instances where the code seems to work as intended, are not hard to
363315rewrite.
364316
365- Conclusion
366- ^^^^^^^^^^
367-
368- The results indicate that ``return ``, ``break `` and ``continue `` in a finally block
369-
370- - are rarely used.
371- - when they are used, they are usually used incorrectly.
372- - code authors are receptive to changing their code, and tend to find it easy to do.
373-
374- ## Acknowledgements
375-
376- I thank:
377-
378- - Alyssa Coghlan for
379- `bringing this issue my attention <https://discuss.python.org/t/pep-760-no-more-bare-excepts/67182/97 >`__.
380-
381- - Guido van Rossum and Hugo van Kemenade for the
382- `script <https://github.com/faster-cpython/tools/blob/main/scripts/download_packages.py >`__
383- that downloads the most popular PyPI packages.
384-
385- - The many code authors I contacted for their responsiveness and grace.
386-
387317Copyright
388318=========
389319
0 commit comments