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

Cleanup unnecessary curframe_locals usage #124369

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

gaogaotiantian
Copy link
Member

This is just a cleanup for the workaround solution for frame locals. Before PEP 667, accessing frame.f_locals will clear the changes pdb made to frame.f_locals so we had to copy it to a new dict and work on that. However, it still has its caveats like #102864.

Now f_locals has a clear meaning and behavior, we should try to use it as much as possible. This change has no user observable behavior change. Theoretically it's a little bit different than before because each call to frame.f_locals creates a new proxy, but in practice we should see no difference.

@ncoghlan could you take a look at the PR? You are familiar with the concept of PEP 667 so I think your review would be helpful. Also it's the sprint and I don't want to throw everything on Irit :)

Copy link
Contributor

@ncoghlan ncoghlan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code simplification LGTM.

While I'm normally paranoid about treating any attribute without a leading underscore as implicitly public (regardless of documentation status), the fact we don't document any public attributes on pdb instances (only methods) makes me more comfortable with it here.

@gaogaotiantian
Copy link
Member Author

Thanks! In general attributes of pdb.Pdb is a bit safer than the other libraries as it's normally used as a tool. Also because it's really old, there's no really "private attribute" until recent changes.

@gaogaotiantian gaogaotiantian merged commit 986a4e1 into python:main Sep 26, 2024
37 checks passed
@gaogaotiantian gaogaotiantian deleted the pdb-cleanup-framelocals branch September 26, 2024 16:35
gabifalk added a commit to gabifalk/gentoo that referenced this pull request Dec 17, 2024
The regression was detected by ipython's own testsuite.

This part of the IPython code is expected to break again with python 3.14,
because the curframe_locals attribute was removed in the PR cpython#124369.
However, there are plans to restore this attribute for backward compatibility in
PR cpython#125951 before the CPython 3.14 release.

Url: ipython/ipython#14598
Url: ipython/ipython@c1e945b
Url: python/cpython#124369
Url: python/cpython#125951
Url: ipython/ipython#14620
Closes: https://bugs.gentoo.org/946568
Signed-off-by: Gabi Falk <gabifalk@gmx.com>
gabifalk added a commit to gabifalk/gentoo that referenced this pull request Dec 17, 2024
The regression was detected by ipython's own testsuite.

This part of the IPython code is expected to break again with python 3.14,
because the curframe_locals attribute was removed in the PR cpython#124369.
However, there are plans to restore this attribute for backward compatibility in
PR cpython#125951 before the CPython 3.14 release.

Url: ipython/ipython#14598
Url: ipython/ipython@c1e945b
Url: python/cpython#124369
Url: python/cpython#125951
Url: ipython/ipython#14620
Closes: https://bugs.gentoo.org/946568
Signed-off-by: Gabi Falk <gabifalk@gmx.com>
gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Dec 20, 2024
The regression was detected by ipython's own testsuite.

This part of the IPython code is expected to break again with python 3.14,
because the curframe_locals attribute was removed in the PR cpython#124369.
However, there are plans to restore this attribute for backward compatibility in
PR cpython#125951 before the CPython 3.14 release.

Url: ipython/ipython#14598
Url: ipython/ipython@c1e945b
Url: python/cpython#124369
Url: python/cpython#125951
Url: ipython/ipython#14620
Closes: https://bugs.gentoo.org/946568
Signed-off-by: Gabi Falk <gabifalk@gmx.com>
Closes: #39746
Signed-off-by: Sam James <sam@gentoo.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants