gh-112205: Support `@setter` annotation from AC by corona10 · Pull Request #112922 · python/cpython

corona10 · 2023-12-10T07:06:16Z

Issue: Support getters and setters in Argument Clinic #112205

Tools/clinic/clinic.py

corona10 · 2023-12-10T07:29:54Z

@colesbury @erlend-aasland @AlexWaygood
The current implementation is not super pretty, so please leave feedback for this implementation.
I tried not to touch the overall architecture of AC as possible.

Unify getter/setter method macro definition with undef keyword.
Update AC to support @setter annotation.

Tools/clinic/clinic.py

corona10 · 2023-12-10T07:32:10Z

For the dev guide, I will send a PR once the PR is landed.

corona10 · 2023-12-10T07:34:22Z

And also, we need to consider supporting PyDoc for getter/setter with the separated PR.

cpython/Modules/_io/textio.c

Lines 181 to 186 in 384d6c1

    
           static PyGetSetDef textiobase_getset[] = { 
        
               {"encoding", (getter)textiobase_encoding_get, NULL, textiobase_encoding_doc}, 
        
               {"newlines", (getter)textiobase_newlines_get, NULL, textiobase_newlines_doc}, 
        
               {"errors", (getter)textiobase_errors_get, NULL, textiobase_errors_doc}, 
        
               {NULL} 
        
           };

erlend-aasland

Overall this looks good to me; thanks!

I would prefer to use a naming aligned with the names already used in the C API (tp_getset, PyGetSetDef); that is, consistently use getset and GETSET instead of getsetter and GETSETTER.

Tools/clinic/clinic.py

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

Tools/clinic/clinic.py

erlend-aasland

Oops, I probably introduced a typo with my recent suggestions; sorry 'bout that!

Tools/clinic/clinic.py

corona10 · 2023-12-11T12:19:44Z

@erlend-aasland Thank you nice catch! PTAL one more time

erlend-aasland

LGTM. Let's wait for Alex to chime in.

(cc. @colesbury, if you'd like to take a look.)

AlexWaygood · 2023-12-12T00:05:41Z

(I'm travelling right now, but will do my best to review this tomorrow or Wednesday!)

AlexWaygood

Thanks! A few small suggestions, mostly to do with error messages:

Tools/clinic/clinic.py

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Tools/clinic/clinic.py

Lib/test/test_clinic.py

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Lib/test/test_clinic.py

AlexWaygood

LGTM, thanks!

--------- Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

pythongh-112205: Support @setter annotation from AC

488a127

corona10 added the skip news label Dec 10, 2023

corona10 requested a review from colesbury December 10, 2023 07:06

corona10 requested review from AlexWaygood and erlend-aasland as code owners December 10, 2023 07:06

bedevere-app bot added the awaiting core review label Dec 10, 2023

bedevere-app bot mentioned this pull request Dec 10, 2023

Support getters and setters in Argument Clinic #112205

Closed

corona10 commented Dec 10, 2023

View reviewed changes

Tools/clinic/clinic.py Show resolved Hide resolved

corona10 changed the title ~~gh-112205: Support @setter annotation from AC~~ [WIP] gh-112205: Support @setter annotation from AC Dec 10, 2023

corona10 marked this pull request as draft December 10, 2023 07:13

bedevere-app bot removed the awaiting core review label Dec 10, 2023

Update

384d6c1

corona10 marked this pull request as ready for review December 10, 2023 07:29

bedevere-app bot added the awaiting core review label Dec 10, 2023

corona10 commented Dec 10, 2023

View reviewed changes

Tools/clinic/clinic.py Show resolved Hide resolved

corona10 changed the title ~~[WIP] gh-112205: Support @setter annotation from AC~~ gh-112205: Support @setter annotation from AC Dec 10, 2023

corona10 added topic-free-threading topic-argument-clinic labels Dec 10, 2023

erlend-aasland reviewed Dec 10, 2023

View reviewed changes

corona10 and others added 5 commits December 10, 2023 19:20

Apply suggestions from code review

2a8c3de

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

Address code review

0ffa78b

nit

8e85ab3

Prohibit declaration of return type for getset

f3517f9

Add test

323cfc0

corona10 requested a review from erlend-aasland December 10, 2023 10:50

corona10 mentioned this pull request Dec 10, 2023

Add "how to" for the setter Argument Clinic directive python/devguide#1245

Merged

erlend-aasland reviewed Dec 11, 2023

View reviewed changes

Tools/clinic/clinic.py Outdated Show resolved Hide resolved

erlend-aasland reviewed Dec 11, 2023

View reviewed changes

Tools/clinic/clinic.py Outdated Show resolved Hide resolved

Tools/clinic/clinic.py Outdated Show resolved Hide resolved

Address code review

d18cd49

corona10 requested a review from erlend-aasland December 11, 2023 13:51

erlend-aasland approved these changes Dec 11, 2023

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Dec 11, 2023

AlexWaygood reviewed Dec 13, 2023

View reviewed changes

corona10 and others added 4 commits December 13, 2023 22:02

Apply suggestions from code review

bc373f6

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Use pattern matching

0cd047b

Address code review

c8a035f

fix test

9c66961

corona10 commented Dec 13, 2023

View reviewed changes

Tools/clinic/clinic.py Show resolved Hide resolved

Add test

06b1cb1

corona10 requested a review from AlexWaygood December 13, 2023 13:19

AlexWaygood reviewed Dec 13, 2023

View reviewed changes

Lib/test/test_clinic.py Show resolved Hide resolved

Address code review

2510a35

AlexWaygood reviewed Dec 13, 2023

View reviewed changes

Lib/test/test_clinic.py Outdated Show resolved Hide resolved

Update Lib/test/test_clinic.py

53777df

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

corona10 requested a review from AlexWaygood December 13, 2023 13:23

AlexWaygood reviewed Dec 13, 2023

View reviewed changes

Lib/test/test_clinic.py Show resolved Hide resolved

Fix

61f7215

AlexWaygood approved these changes Dec 13, 2023

View reviewed changes

corona10 enabled auto-merge (squash) December 13, 2023 13:33

corona10 merged commit 498a096 into python:main Dec 13, 2023

bedevere-app bot removed the awaiting merge label Dec 13, 2023

corona10 deleted the gh-112205-getset branch December 13, 2023 14:00

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024

pythongh-112205: Support @setter annotation from AC (pythongh-112922)

6ad31bb

--------- Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Uh oh!

Conversation

corona10 commented Dec 10, 2023 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

corona10 commented Dec 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

corona10 commented Dec 10, 2023

Uh oh!

corona10 commented Dec 10, 2023

Uh oh!

erlend-aasland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erlend-aasland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

corona10 commented Dec 11, 2023

Uh oh!

erlend-aasland left a comment

Choose a reason for hiding this comment

Uh oh!

AlexWaygood commented Dec 12, 2023

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

corona10 commented Dec 10, 2023 •

edited by bedevere-app bot

Loading

corona10 commented Dec 10, 2023 •

edited

Loading