Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-112205: Support docstring for @getter#113160
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
corona10 commented Dec 15, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@erlend-aasland@AlexWaygood I'm sharing my consideration for the design of this PR.
I will update the devguide about the restriction of docstring for property. |
Uh oh!
There was an error while loading. Please reload this page.
corona10 commented Dec 15, 2023
@erlend-aasland@AlexWaygood Updated :) PTAL |
erlend-aasland commented Dec 18, 2023
Ideally, I would prefer this:
What do you think, @AlexWaygood? |
corona10 commented Dec 18, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Ideally, yes, but I think that this very difficult requirement in current status if you want to support all of the situations in a AC; since the AC does not care about another method state (aka stateless), it's very difficult to interact with another method or propety state. |
corona10 commented Dec 18, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
If we want to make AC care about other method states, we should reconstruct AC from scratch. |
corona10 commented Dec 18, 2023
Note: The reason I used the macro declaration of generated c codes was that it was very easy to care about other property declaration statuses without touching overall AC architecture. And the generated C code obviously not affected to performance issue at all :) |
corona10 commented Dec 18, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Workaround for |
vstinner commented Dec 18, 2023
This generated code is verbose, but I read AC code, and I know that it's tricky to add new features in a safe way. PyDoc_STRVAR(_io__TextIOBase_encoding__doc__, "Encoding of the text stream.\n""\n""Subclasses should override."); #define_io__TextIOBase_encoding_HAS_DOCSTR#if defined(_io__TextIOBase_encoding_HAS_DOCSTR) # define_io__TextIOBase_encoding_DOCSTR _io__TextIOBase_encoding__doc__ #else# define_io__TextIOBase_encoding_DOCSTR NULL #endifI would prefer to not hold this nice enhancement until AC is refactored in a way which avoids such verbose code. Since the code is is generated, I don't care too much about the details, as soon as the C code, after the preprocessor is passed, remains efficient :-) |
AlexWaygood commented Dec 18, 2023
I'll defer to you three on this one; I agree that @erlend-aasland's idea sounds nicer in principle, but I also don't mind there being slightly ugly generated C code for now, as long as it results in the same behaviour for Python users :) |
erlend-aasland commented Dec 18, 2023
If there is a need to get this in very quick, I'm ok with rushing it. A lot of the setter/getter code is special cased, mostly because clinic was designed for
Well, clinic is already very flexible, so we might not need to refactor it at all, in order to make it work more smoothly. |
corona10 commented Dec 18, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@erlend-aasland@AlexWaygood@vstinner
Okay, negotiation time :)
Here is a note that I considered cases for this issue that you should consider if you want to suggest other designs.
|
erlend-aasland commented Dec 19, 2023
As I already wrote, I'm fine with proceeding with this implementation as a temporary workaround. IMO, I think we (myself included) should have put more time into the design of the PyGetSetDef feature before starting to land the recent clinic PRs. If Alex and Victor are ok with this PR, I won't block it. But please create a follow-up issue for refactoring (and possibly redesigning) the PyGetSetDef feature of Argument Clinic, so we don't forget about that :) |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Uh oh!
There was an error while loading. Please reload this page.
vstinner commented Dec 20, 2023
So far, I avoided "large refactoring" of |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
erlend-aasland left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks; please wait for Alex's thumbs up before landing.
corona10 commented Dec 20, 2023
@AlexWaygood ^^^^ :) |
AlexWaygood left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @corona10 <3
--------- Co-authored-by: Erlend E. Aasland <erlend@python.org>
--------- Co-authored-by: Erlend E. Aasland <erlend@python.org>
--------- Co-authored-by: Erlend E. Aasland <erlend@python.org>
Uh oh!
There was an error while loading. Please reload this page.