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

gh-95065: Add Argument Clinic support for deprecating positional use of parameters #95151

Merged
merged 95 commits into from
Aug 7, 2023

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jul 22, 2022

Add support for deprecating positional use of optional parameters by
introducing the * [from X.Y] syntax, meaning the following parameters
will be keyword-only starting with Python version X.Y. Code that emits
DeprecationWarnings and compile time messages will be automatically
generated.

Multiple * [from X.Y] lines are not allowed (yet).

@erlend-aasland
Copy link
Contributor Author

Disclaimer: there's probably a lot of corner cases I haven't thought of yet. This is very much a draft.

@erlend-aasland erlend-aasland marked this pull request as ready for review July 26, 2022 11:45
@erlend-aasland
Copy link
Contributor Author

@zware, are you interested in reviewing this?

This is probably quite crude, but my Argument Clinic knowledge is minimal, so I think getting a round of reviews is needed for me to move forward with this.

@erlend-aasland
Copy link
Contributor Author

cc. @arhadthedev, if you would like to review this.

arhadthedev
arhadthedev previously approved these changes Jul 26, 2022
Copy link
Member

@arhadthedev arhadthedev left a comment

Choose a reason for hiding this comment

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

if you would like to review this

I have neither objections nor extra ideas here. Also, I fully agree that [...] we should create separate issues for each feature later, not here and now.

@erlend-aasland erlend-aasland dismissed arhadthedev’s stale review July 2, 2023 21:48

The feature is not ready yet; dismissing premature approval.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM, besides few nitpicks.

Tools/clinic/clinic.py Outdated Show resolved Hide resolved
Tools/clinic/clinic.py Outdated Show resolved Hide resolved
Tools/clinic/clinic.py Outdated Show resolved Hide resolved
@erlend-aasland
Copy link
Contributor Author

LGTM, besides few nitpicks.

Thank you all so much for helping out with this PR. I learned some new tricks, and it was very good with some extra pairs of eyes on this.

@erlend-aasland
Copy link
Contributor Author

I improved the NEWS entry, but I also think we should promote this feature in What's New. I'm not sure where in What's New, though.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

No, I do not think that it should be in What's New. Argument Clinic is an implementation detail. Everything made with Argument Clinic can be made without Argument Clinic. Only the end result is important for users.

…5.NfCCpp.rst

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@erlend-aasland
Copy link
Contributor Author

Thanks again. I'll wait for Alex's approval before landing.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

The grammar in one of the deprecation messages in the tests is still not ideal — here's a way to fix it:

Lib/test/clinic.test.c Outdated Show resolved Hide resolved
Tools/clinic/clinic.py Outdated Show resolved Hide resolved
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

LGTM

@erlend-aasland erlend-aasland enabled auto-merge (squash) August 7, 2023 10:40
This reverts commit d2130ab,
but keeps the test.
@erlend-aasland erlend-aasland enabled auto-merge (squash) August 7, 2023 11:01
@erlend-aasland erlend-aasland merged commit 33cb0b0 into python:main Aug 7, 2023
18 checks passed
@erlend-aasland erlend-aasland deleted the ac-deprecate branch August 7, 2023 11:28
erlend-aasland added a commit to erlend-aasland/devguide that referenced this pull request Sep 8, 2023
…of parameters (python/cpython#95151)

It is now possible to deprecate passing parameters positionally with
Argument Clinic, using the new '* [from X.Y]' syntax.
(To be read as "keyword-only from Python version X.Y")

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
AA-Turner pushed a commit to AA-Turner/devguide that referenced this pull request Sep 13, 2023
…itional use of parameters (python/cpython#95151)

It is now possible to deprecate passing parameters positionally with
Argument Clinic, using the new '* [from X.Y]' syntax.
(To be read as "keyword-only from Python version X.Y")

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
erlend-aasland added a commit to python/devguide that referenced this pull request Sep 26, 2023
…itional use of parameters (python/cpython#95151)

It is now possible to deprecate passing parameters positionally with
Argument Clinic, using the new '* [from X.Y]' syntax.
(To be read as "keyword-only from Python version X.Y")

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
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.

7 participants