Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gui: Solve a recursion error in gui/wxpython/lmgr/giface.py #4514

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

echoix
Copy link
Member

@echoix echoix commented Oct 14, 2024

Fixes #4485 (comment)

The RecursionError is probably caused by #4453, where one of these RecursionErrors was caught in tests, and fixed in a commit in that PR (f7874f6). The exact reason causing the failure wasn't known, so fixing the method mapsets that might have been called elsewhere was the correct thing. There wasn't the same pattern in the PR to be fixed.

However, with that comment, it is now clear that it is calling list() inside __len__() of that same object that is problematic. After reading a bit, the list constructor gets the length or the length hint (when available) as an optimization to allocate memory.
https://stackoverflow.com/questions/41474829/why-does-list-ask-about-len https://github.com/python/cpython/blob/67f6e08147bc005e460d82fcce85bf5d56009cf5/Objects/listobject.c#L1164

I've worked on creating a repro case and filed an issue to Ruff astral-sh/ruff#13752 about that, as the explanations on why the rule was considered an unsafe fix was misleading, as it only concerned lost comments.

@echoix echoix added this to the 8.5.0 milestone Oct 14, 2024
@echoix echoix enabled auto-merge (squash) October 14, 2024 15:45
@github-actions github-actions bot added GUI wxGUI related Python Related code is in Python labels Oct 14, 2024
@echoix echoix merged commit e139a4c into OSGeo:main Oct 14, 2024
26 of 27 checks passed
@echoix echoix deleted the layerlist-len branch October 14, 2024 17:06
@neteler
Copy link
Member

neteler commented Oct 15, 2024

@echoix Should this be backported to G8.4?

@echoix
Copy link
Member Author

echoix commented Oct 15, 2024

No, it was caused by a recent PR that wasn't backported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI wxGUI related Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to compile GRASS GIS on Windows
3 participants