gh-112205: Support `@setter` annotation from AC#112922

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

staticPyGetSetDeftextiobase_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@setterannotation 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-appbot added the awaiting core review label Dec 10, 2023

bedevere-appbot 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.pyShow resolvedHide resolved

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

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

bedevere-appbot 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-appbot added the awaiting core review label Dec 10, 2023

corona10 commented Dec 10, 2023
View reviewed changes

Tools/clinic/clinic.pyShow resolvedHide resolved

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

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

erlend-aasland reviewed Dec 10, 2023
View reviewed changes

corona10and 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 resolvedHide resolved

erlend-aasland reviewed Dec 11, 2023
View reviewed changes

Tools/clinic/clinic.py Outdated Show resolvedHide resolved
Tools/clinic/clinic.py Outdated Show resolvedHide 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-appbot added awaiting merge and removed awaiting core review labels Dec 11, 2023

AlexWaygood reviewed Dec 13, 2023
View reviewed changes

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

corona10and 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.pyShow resolvedHide 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.pyShow resolvedHide resolved

Address code review
2510a35

AlexWaygood reviewed Dec 13, 2023
View reviewed changes

Lib/test/test_clinic.py Outdated Show resolvedHide 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.pyShow resolvedHide 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:mainDec 13, 2023

bedevere-appbot 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@setterannotation from AC (pythongh-112922)
6ad31bb
--------- Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
pythongh-112205: Support@setterannotation from AC (pythongh-112922)
2475e03
--------- Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Uh oh!

gh-112205: Support @setter annotation from AC#112922

gh-112205: Support @setter annotation from AC #112922

Uh oh!

Conversation

corona10 commented Dec 10, 2023• edited by bedevere-app botLoading 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

gh-112205: Support `@setter` annotation from AC#112922

gh-112205: Support `@setter` annotation from AC #112922

corona10 commented Dec 10, 2023•
edited by bedevere-app bot
Loading

corona10 commented Dec 10, 2023•
edited
Loading