Title: Submodule.iter_items sometimes attaches second empty iterator to its StopIteration · Issue #1779 · gitpython-developers/GitPython · GitHub
Open Graph Title: Submodule.iter_items sometimes attaches second empty iterator to its StopIteration · Issue #1779 · gitpython-developers/GitPython
X Title: Submodule.iter_items sometimes attaches second empty iterator to its StopIteration · Issue #1779 · gitpython-developers/GitPython
Description: This is minor bug, and the main benefit of fixing it may be an improvement in code clarity rather than the behavioral difference. I think it makes sense to fix this in the same pull request that fixes #1775, and I've opened #1780 for tha...
Open Graph Description: This is minor bug, and the main benefit of fixing it may be an improvement in code clarity rather than the behavioral difference. I think it makes sense to fix this in the same pull request that fi...
X Description: This is minor bug, and the main benefit of fixing it may be an improvement in code clarity rather than the behavioral difference. I think it makes sense to fix this in the same pull request that fi...
Opengraph URL: https://github.com/gitpython-developers/GitPython/issues/1779
X: @github
Domain: github.com
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Submodule.iter_items sometimes attaches second empty iterator to its StopIteration","articleBody":"*This is minor bug, and the main benefit of fixing it may be an improvement in code clarity rather than the behavioral difference. I think it makes sense to fix this in the same pull request that fixes #1775, and I've opened #1780 for that, but I think it's clearer to document it here than in the pull request.*\r\n\r\n`Submodule.iter_items` is a generator function as intended, because `yield` appears in its scope:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/d986a59467e586afb815d71845f29b605b756cdf/git/objects/submodule/base.py#L1445-L1446\r\n\r\nSometimes when one writes a function that returns a generator object, it is a regular function rather than a generator function, because the function returns a generator expression or because it calls another function that returns a generator object. That technique is sometimes used to fail fast on errors, since calling a generator function does not immediately run its code, but only returns a generator object, and the code runs when that generator object is iterated. However, that is not done here. That it is not appears intentional and reasonable, and even if not, changing it would probably break backwards compatibility.\r\n\r\nHowever, it contains this code, at the top:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/d986a59467e586afb815d71845f29b605b756cdf/git/objects/submodule/base.py#L1400-L1405\r\n\r\nThe intention there is to make it so that calling `iter_items` returns an empty iterator when either of those two exceptions is raised. It succeeds at doing that, but its success is unrelated to using `iter([])` as the operand of `return`. Returning from a generator function--like falling off the end, which is more common--causes there to be no more items to iterate through, so a `next` call in which this happens raises `StopIteration`.\r\n\r\nThe operand of `return` in a generator function is neither returned to the caller--a generator object is returned--nor returned when `next` is called on that generator object. Instead, it becomes *the `value` attribute* of the `StopIteration` exception raised on the `next` call to indicate there are no more values. This feature of generators is rarely used, and the common ways of using generators, such as a `for` loop, ignore it. (When `yield from` is used as an expression, it evaluates to that value, which allows generators to get status information from their subgenerators, in the rare case their subgenerators return a meaningful value. It can also be accessed directly by explicitly catching `StopIteration`, binding the exception object to a variable, and accessing its `value` attribute. That's about it.)\r\n\r\nI suppose it might make sense for `Submodule.iter_items` to have its returned generator object signal, in some way, that it is empty due to `OSError` (`IOError` and `OSError` are the same in Python 3) or `BadName`. But I think that having it attach a second empty iterator object to the `StopIteration` exception is clearly not intended to do so.\r\n\r\nThe statement was originally a bare `return`. The `iter([])` operand was added in 82b131c (#1282). It came in along with numerous additions and improvements in type hinting, and it looks like it was mistakenly added with the idea it might be needed for make type-checking succeed. However, I don't believe the major type checkers were as stable back then as they are today, so perhaps it was instead added as a workaround for a bug in a static type checker.\r\n\r\nInterestingly, this is conceptually related to #1755, and I would've fixed it there, had I been aware of it at that time. The logic in the body `Submodule.iter_items` usually implicitly \"returns\" `None` by falling off the end (and `None` is bound as the `StopIteration` exception's `value` attribute), which is its intended behavior, but sometimes it inadvertently \"returns\" a `list_iterator` object instead.","author":{"url":"https://github.com/EliahKagan","@type":"Person","name":"EliahKagan"},"datePublished":"2023-12-22T09:53:07.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":2},"url":"https://github.com/1779/GitPython/issues/1779"}
| route-pattern | /_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format) |
| route-controller | voltron_issues_fragments |
| route-action | issue_layout |
| fetch-nonce | v2:33c49df5-9097-1bf2-7c78-82493485f652 |
| current-catalog-service-hash | 81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114 |
| request-id | C2AE:D065:77E4F:A1F21:69681904 |
| html-safe-nonce | d999b78b4c11b237b82cf67a05b805b4ae0cd8973b55e3e0c8c8938c26c1dd12 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJDMkFFOkQwNjU6NzdFNEY6QTFGMjE6Njk2ODE5MDQiLCJ2aXNpdG9yX2lkIjoiMzM4MzU0NjI4NTE4MjY4NzQ5MyIsInJlZ2lvbl9lZGdlIjoiaWFkIiwicmVnaW9uX3JlbmRlciI6ImlhZCJ9 |
| visitor-hmac | 025698d6c8038c2d47355cf79889fb13d84e6653be8336565bd2b06dc25b6d58 |
| hovercard-subject-tag | issue:2053669402 |
| github-keyboard-shortcuts | repository,issues,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/gitpython-developers/GitPython/1779/issue_layout |
| twitter:image | https://opengraph.githubassets.com/1eca1511cd843ab0132567d57ba042b2574578d4230df91a6cf8424ef888bfe0/gitpython-developers/GitPython/issues/1779 |
| twitter:card | summary_large_image |
| og:image | https://opengraph.githubassets.com/1eca1511cd843ab0132567d57ba042b2574578d4230df91a6cf8424ef888bfe0/gitpython-developers/GitPython/issues/1779 |
| og:image:alt | This is minor bug, and the main benefit of fixing it may be an improvement in code clarity rather than the behavioral difference. I think it makes sense to fix this in the same pull request that fi... |
| og:image:width | 1200 |
| og:image:height | 600 |
| og:site_name | GitHub |
| og:type | object |
| og:author:username | EliahKagan |
| hostname | github.com |
| expected-hostname | github.com |
| None | 2452ef75cc61e096c7ab0f8a4fca0ff7d45789bc2e145182d98e41a6ba61e5e1 |
| turbo-cache-control | no-preview |
| go-import | github.com/gitpython-developers/GitPython git https://github.com/gitpython-developers/GitPython.git |
| octolytics-dimension-user_id | 503709 |
| octolytics-dimension-user_login | gitpython-developers |
| octolytics-dimension-repository_id | 1126087 |
| octolytics-dimension-repository_nwo | gitpython-developers/GitPython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 1126087 |
| octolytics-dimension-repository_network_root_nwo | gitpython-developers/GitPython |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 424b2f45b6311b8635b559db771ca199ff3a1b4f |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width