Skip to content

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Dec 3, 2023

@vkhodygo
Copy link

Any progress?

@sobolevn sobolevn requested review from AlexWaygood and hugovk March 9, 2024 08:01
@sobolevn
Copy link
Member Author

sobolevn commented Mar 9, 2024

@hugovk I've implemented your suggestion about porting to Python 3.13, please take a look.
@AlexWaygood We discussed this deprecation in #112279

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Please start with a separated PR which adds Signature.from_frame(). Once it will be merge, you can simplify this PR to only deprecate methods.

Comment on lines +3123 to +3125
for name in arg_names[:pos_count]:
if frame.f_locals and name in frame.f_locals:
defaults.append(frame.f_locals[name])
Copy link
Member

Choose a reason for hiding this comment

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

It seems like you can move if frame.f_locals: out of this loop and the one below.

@hugovk
Copy link
Member

hugovk commented Mar 9, 2024

(Copying and pasting my reply from elsewhere :)

PEP 387 says two releases of deprecation is the minimum but also:

  • If the deprecated feature is replaced by a new one, it should generally be removed only after the last Python version without the new feature reaches end of support.

Does that apply here? If so, and if the replacement is added in 3.13, then the removals should be when 3.13 is EOL in October 2029, meaning removal in 3.18, not 3.15.

class TestSignatureFromFrame(unittest.TestCase):
def test_signature_from_frame(self):
def inner(a=1, /, b=2, *e, c: int = 3, d, **f) -> None:
global fr
Copy link
Member

Choose a reason for hiding this comment

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

Using a global sounds like a bad idea. You can use a "nonlocal" instead.

Sometimes, I use a mutable type instead, which is more or less the same:

ns = {}
def func():
    ns['name'] = value

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.

4 participants