GeezerVison is showing github.com — GH-137630: Convert ``_interpqueues`` to use Argument Clinic by AA-Turner · Pull Request #137685 · python/cpython
Conversation
AA-Turner
commented
Aug 12, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Note, we use a custom converter qidarg_converter here. I have changed the qidarg_converter() function to simplify the return -- we only ever use qidarg.id.
pydoc diff:
--- pydoc-interpqueues-HEAD.txt 2025-08-12 19:29:03.554066000 +0100
+++ pydoc-interpqueues-AC.txt 2025-08-12 19:26:19.952112000 +0100
@@ -130,70 +130,52 @@
| args
FUNCTIONS
- bind(...)
- bind(qid)
-
+ bind(qid)
Take a reference to the identified queue.
- The queue is not destroyed until there are no references left.
- create(...)
- create(maxsize, unboundop, fallback) -> qid
+ The queue is not destroyed until there are no references left.
+ create(maxsize, unboundop=-1, fallback=-1)
Create a new cross-interpreter queue and return its unique generated ID.
- It is a new reference as though bind() had been called on the queue.
+ It is a new reference as though bind() had been called on the queue.
The caller is responsible for calling destroy() for the new queue
before the runtime is finalized.
- destroy(...)
- destroy(qid)
+ destroy(qid)
+ Clear and destroy the queue.
- Clear and destroy the queue. Afterward attempts to use the queue
- will behave as though it never existed.
-
- get(...)
- get(qid) -> (obj, unboundop)
+ Afterward attempts to use the queue will behave as though it never existed.
+ get(qid)
Return a new object from the data at the front of the queue.
- The unbound op is also returned.
+ The unbound op is also returned.
If there is nothing to receive then raise QueueEmpty.
- get_count(...)
- get_count(qid)
-
+ get_count(qid)
Return the number of items in the queue.
- get_maxsize(...)
- get_maxsize(qid)
-
+ get_maxsize(qid)
Return the maximum number of items in the queue.
- get_queue_defaults(...)
- get_queue_defaults(qid)
-
+ get_queue_defaults(qid)
Return the queue's default values, set when it was created.
- is_full(...)
- is_full(qid)
-
+ is_full(qid)
Return true if the queue has a maxsize and has reached it.
list_all()
- list_all() -> [(qid, unboundop, fallback)]
-
Return the list of IDs for all queues.
- Each corresponding default unbound op and fallback is also included.
- put(...)
- put(qid, obj)
+ Each corresponding default unbound op and fallback is also included.
+ put(qid, obj, unboundop=-1, fallback=-1)
Add the object's data to the queue.
- release(...)
- release(qid)
-
+ release(qid)
Release a reference to the queue.
+
The queue is destroyed once there are no references left.
FILE
A
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
There is a bug in the current code which should be fixed in 3.14 too. The rest LGTM. But I would like to get @ericsnowcurrently's approve.
Sorry, something went wrong.
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.
|
I also add that after removing the return pseudo-annotation, some docstrings look more self-contadicting.
something(qid)
Return bla-bla X.
Ah, forget it, also return Y and Z.
It was in the old docstrings, but with -> (X, Y, Z) you at least could suspect that "return X" was not the whole truth.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
# Conflicts:
# Modules/_interpqueuesmodule.c
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.
Technically LGTM, but what would @ericsnowcurrently say?
It is sad that this cannot be backported to 3.14, so there will be such large difference between 3.14 and 3.15.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
Considering everything stays the same functionally (per all the passing tests), I don't really have any meaningful objections, especially since this is an internal module where I do not expect anyone to ever look at the docstrings. In fact, we could probably change all the docstrings to comments.
FWIW, my review comments are mostly me fussing about argument clinic, which is probably way too late to be relevant. :)
Sorry, something went wrong.
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.
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.
|
Thanks both for the reviews!
A
|
ericsnowcurrently reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|