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-112075: Adapt more dict methods to Argument Clinic #114256

Merged
merged 13 commits into from
Jan 23, 2024
Merged

Conversation

DinoV
Copy link
Contributor

@DinoV DinoV commented Jan 18, 2024

Uses argument clinic for the remaining dict methods which aren't using it yet. This will allow to use argument clinic's @critical_section consistently where it's appropriate.

Mostly straight forward, the most dramatic thing is that some of the argument clinic names overlap with existing names. Those existing names are just wrapped in public dict API so the functionality just moves into the public API.

Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
@DinoV DinoV force-pushed the nogil_dict branch 2 times, most recently from 5e7a200 to 72a78e4 Compare January 19, 2024 21:02
@rhettinger rhettinger removed their request for review January 19, 2024 21:28
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
@erlend-aasland
Copy link
Contributor

Sorry to add remarks on the already amended docstrings, but let's avoid the CI churn of addressing this in another PR :)

@erlend-aasland erlend-aasland changed the title gh-112075: Move more dict objects to argument clinic in preparation for using argument clinic for more locking gh-112075: Adapt more dict methods to Argument Clinic Jan 23, 2024
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

LGTM with last round of suggestions applied.

DinoV and others added 3 commits January 23, 2024 12:57
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
DinoV and others added 4 commits January 23, 2024 12:58
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@DinoV
Copy link
Contributor Author

DinoV commented Jan 23, 2024

Thanks, I've appied them all!

@DinoV DinoV enabled auto-merge (squash) January 23, 2024 20:59
@erlend-aasland
Copy link
Contributor

Thanks, I've appied them all!

Regen clinic ;)

@DinoV DinoV merged commit afe8f37 into python:main Jan 23, 2024
30 checks passed
@erlend-aasland
Copy link
Contributor

@DinoV, next time, can you please clean up the commit message before merging?

@DinoV
Copy link
Contributor Author

DinoV commented Jan 23, 2024

@DinoV, next time, can you please clean up the commit message before merging?

Ahh yes, sorry! :(

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

* Move more dict objects to argument clinic

* Improve doc strings

* More doc string improvements

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

---------

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…14256)

* Move more dict objects to argument clinic

* Improve doc strings

* More doc string improvements

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

* Update Objects/dictobject.c

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

---------

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.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.

3 participants